Permalink
Browse files

use arel rather than slapping together SQL strings

  • Loading branch information...
tenderlove authored and senny committed Jul 15, 2013
1 parent 9c98535 commit 83cc36d96387f833e5ecdae4a346720377b57877
View
@@ -1,5 +1,18 @@
## 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
Example:
@@ -285,8 +285,10 @@ def order!(*args) # :nodoc:
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
+ args = args.map! { |arg|
+ arg.is_a?(Symbol) ?
+ Arel::Nodes::Ascending.new(klass.arel_table[arg]) :
+ arg
}
self.order_values += args
@@ -211,8 +211,8 @@ def test_relation_merging_with_merged_joins_as_strings
class RelationMutationTest < ActiveSupport::TestCase
class FakeKlass < Struct.new(:table_name, :name)
- def quoted_table_name
- %{"#{table_name}"}
+ def arel_table
+ Post.arel_table
end
end
@@ -234,7 +234,10 @@ def relation
test "#order! with symbol prepends the table name" do
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
test '#references!' do
@@ -177,6 +177,10 @@ def test_finding_with_reverted_assoc_order
assert_equal topics(:fourth).title, topics.first.title
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
assert_raise(ArgumentError) { Topic.order(:name, "id DESC", :id => :DeSc) }
end
@@ -1570,7 +1574,7 @@ def __omg__
assert merged.to_sql.include?("wtf")
assert merged.to_sql.include?("bbq")
end
-
+
def test_merging_removes_lhs_bind_parameters
left = Post.where(id: Arel::Nodes::BindParam.new('?'))
column = Post.columns_hash['id']

0 comments on commit 83cc36d

Please sign in to comment.