Skip to content

Commit

Permalink
Merge pull request #11880 from senny/backport_arel_fix
Browse files Browse the repository at this point in the history
Backport arel fix
  • Loading branch information
senny committed Oct 14, 2013
2 parents 9c98535 + 83cc36d commit 146132d
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
13 changes: 13 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Original file line Diff line number Diff line change
@@ -1,5 +1,18 @@
## unreleased ## ## unreleased ##


* `Relation#order` quotes the column name if you pass a `Symbol`.
Fixes #11870.

Example:

# Before
Post.order(:id).to_sql == '... ORDER BY "posts".id ASC'

# After
Post.order(:id).to_sql == '... ORDER BY "posts"."id" ASC'

*Yves Senn*

* Generate subquery for `Relation` if it passed as array condition for `where` method * Generate subquery for `Relation` if it passed as array condition for `where` method


Example: Example:
Expand Down
6 changes: 4 additions & 2 deletions activerecord/lib/active_record/relation/query_methods.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -285,8 +285,10 @@ def order!(*args) # :nodoc:
references!(references) if references.any? references!(references) if references.any?


# if a symbol is given we prepend the quoted table name # if a symbol is given we prepend the quoted table name
args = args.map { |arg| args = args.map! { |arg|
arg.is_a?(Symbol) ? "#{quoted_table_name}.#{arg} ASC" : arg arg.is_a?(Symbol) ?
Arel::Nodes::Ascending.new(klass.arel_table[arg]) :
arg
} }


self.order_values += args self.order_values += args
Expand Down
9 changes: 6 additions & 3 deletions activerecord/test/cases/relation_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ def test_relation_merging_with_merged_joins_as_strings


class RelationMutationTest < ActiveSupport::TestCase class RelationMutationTest < ActiveSupport::TestCase
class FakeKlass < Struct.new(:table_name, :name) class FakeKlass < Struct.new(:table_name, :name)
def quoted_table_name def arel_table
%{"#{table_name}"} Post.arel_table
end end
end end


Expand All @@ -234,7 +234,10 @@ def relation


test "#order! with symbol prepends the table name" do test "#order! with symbol prepends the table name" do
assert relation.order!(:name).equal?(relation) assert relation.order!(:name).equal?(relation)
assert_equal ['"posts".name ASC'], relation.order_values node = relation.order_values.first
assert node.ascending?
assert_equal :name, node.expr.name
assert_equal "posts", node.expr.relation.name
end end


test '#references!' do test '#references!' do
Expand Down
6 changes: 5 additions & 1 deletion activerecord/test/cases/relations_test.rb
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -177,6 +177,10 @@ def test_finding_with_reverted_assoc_order
assert_equal topics(:fourth).title, topics.first.title assert_equal topics(:fourth).title, topics.first.title
end end


def test_order_with_hash_and_symbol_generates_the_same_sql
assert_equal Topic.order(:id).to_sql, Topic.order(id: :asc).to_sql
end

def test_raising_exception_on_invalid_hash_params def test_raising_exception_on_invalid_hash_params
assert_raise(ArgumentError) { Topic.order(:name, "id DESC", :id => :DeSc) } assert_raise(ArgumentError) { Topic.order(:name, "id DESC", :id => :DeSc) }
end end
Expand Down Expand Up @@ -1570,7 +1574,7 @@ def __omg__
assert merged.to_sql.include?("wtf") assert merged.to_sql.include?("wtf")
assert merged.to_sql.include?("bbq") assert merged.to_sql.include?("bbq")
end end

def test_merging_removes_lhs_bind_parameters def test_merging_removes_lhs_bind_parameters
left = Post.where(id: Arel::Nodes::BindParam.new('?')) left = Post.where(id: Arel::Nodes::BindParam.new('?'))
column = Post.columns_hash['id'] column = Post.columns_hash['id']
Expand Down

0 comments on commit 146132d

Please sign in to comment.