From e0356990856abc9a84e6e038e7b06e2931502728 Mon Sep 17 00:00:00 2001
From: Yves Senn <yves.senn@gmail.com>
Date: Mon, 25 Feb 2013 11:40:30 +0100
Subject: Expand order(:symbol) to "table".symbol to prevent broken queries on
 PG. Fixes #9275.

When `#order` is called with a Symbol this patch will prepend the quoted_table_name.
Before the postgresql adapter failed to build queries containg a join and an order
with a symbol.

This expansion happens for all adapters.
---
 activerecord/CHANGELOG.md                               |  9 +++++++++
 .../lib/active_record/relation/query_methods.rb         |  5 +++++
 activerecord/test/cases/associations/eager_test.rb      |  7 ++++++-
 activerecord/test/cases/relation_test.rb                | 17 +++++++++++++++--
 4 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index c7085a0eaa..3b61b98629 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,14 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Fix when performing an ordered join query. The bug only
+    affected queries where the order was given with a symbol.
+    Fixes #9275.
+
+    Example:
+
+        # This will expand the order :name to "authors".name.
+        Author.joins(:books).where('books.published = 1').order(:name)
+
 *   Fixing issue #8345. Now throwing an error when one attempts to touch a
     new object that has not yet been persisted. For instance:
 
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 225677085f..4b8c40592e 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -285,6 +285,11 @@ module ActiveRecord
       references.map! { |arg| arg =~ /^([a-zA-Z]\w*)\.(\w+)/ && $1 }.compact!
       references!(references) if references.any?
 
+      # if a symbol is given we prepend the quoted table name
+      args = args.map { |arg|
+        arg.is_a?(Symbol) ? "#{quoted_table_name}.#{arg} ASC" : arg
+      }
+
       self.order_values = args + self.order_values
       self
     end
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 46f3c38ac5..1de7ee0846 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -218,7 +218,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
   def test_finding_with_includes_on_null_belongs_to_association_with_same_include_includes_only_once
     post = posts(:welcome)
     post.update!(author: nil)
-    post = assert_queries(1) { Post.all.merge!(includes: {author_with_address: :author_address}).find(post.id) } 
+    post = assert_queries(1) { Post.all.merge!(includes: {author_with_address: :author_address}).find(post.id) }
     # find the post, then find the author which is null so no query for the author or address
     assert_no_queries do
       assert_equal nil, post.author_with_address
@@ -1173,4 +1173,9 @@ class EagerAssociationTest < ActiveRecord::TestCase
     assert_no_queries { assert_equal 2, author.comments_with_order_and_conditions.size }
     assert_no_queries { assert_equal 5, author.posts.size, "should not cache a subset of the association" }
   end
+
+  test "works in combination with order(:symbol)" do
+    author = Author.includes(:posts).references(:posts).order(:name).where('posts.title IS NOT NULL').first
+    assert_equal authors(:bob), author
+  end
 end
diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb
index 92dc575d37..fd0b05cb77 100644
--- a/activerecord/test/cases/relation_test.rb
+++ b/activerecord/test/cases/relation_test.rb
@@ -180,19 +180,32 @@ module ActiveRecord
 
   class RelationMutationTest < ActiveSupport::TestCase
     class FakeKlass < Struct.new(:table_name, :name)
+      def quoted_table_name
+        %{"#{table_name}"}
+      end
     end
 
     def relation
-      @relation ||= Relation.new FakeKlass, :b
+      @relation ||= Relation.new FakeKlass.new('posts'), :b
     end
 
-    (Relation::MULTI_VALUE_METHODS - [:references, :extending]).each do |method|
+    (Relation::MULTI_VALUE_METHODS - [:references, :extending, :order]).each do |method|
       test "##{method}!" do
         assert relation.public_send("#{method}!", :foo).equal?(relation)
         assert_equal [:foo], relation.public_send("#{method}_values")
       end
     end
 
+    test "#order!" do
+      assert relation.order!('name ASC').equal?(relation)
+      assert_equal ['name ASC'], relation.order_values
+    end
+
+    test "#order! with symbol prepends the table name" do
+      assert relation.order!(:name).equal?(relation)
+      assert_equal ['"posts".name ASC'], relation.order_values
+    end
+
     test '#references!' do
       assert relation.references!(:foo).equal?(relation)
       assert relation.references_values.include?('foo')
-- 
cgit v1.2.3