diff options
| author | Ryuta Kamizono <kamipo@gmail.com> | 2018-08-20 23:41:45 +0900 |
|---|---|---|
| committer | Ryuta Kamizono <kamipo@gmail.com> | 2018-08-21 00:01:38 +0900 |
| commit | 96cd16bdeec661c9ecf1a83ca41a2cb22f435af9 (patch) | |
| tree | e13a9198b23dcffc75c1588a1fd51fe936c45968 /activerecord | |
| parent | ffca883908432f61a1fefdbcef5b1c10a7223acb (diff) | |
| download | rails-96cd16bdeec661c9ecf1a83ca41a2cb22f435af9.tar.gz rails-96cd16bdeec661c9ecf1a83ca41a2cb22f435af9.tar.bz2 rails-96cd16bdeec661c9ecf1a83ca41a2cb22f435af9.zip | |
Fix merging relation that order including `?`
The `Relation::Merger` has a problem that order values would be merged
as nested array.
That was caused an issue #33664 since if array value is passed to
`order` and first element in the array includes `?`, the array is
regarded as a prepared statement and bind variables.
https://api.rubyonrails.org/classes/ActiveRecord/Sanitization/ClassMethods.html#method-i-sanitize_sql_for_order
Just merging that as splat args like other values would fix the issue.
Fixes #33664.
Diffstat (limited to 'activerecord')
| -rw-r--r-- | activerecord/lib/active_record/relation/merger.rb | 4 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/merging_test.rb | 10 |
2 files changed, 12 insertions, 2 deletions
diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 10e4779ca4..07b16a0740 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -152,10 +152,10 @@ module ActiveRecord def merge_multi_values if other.reordering_value # override any order specified in the original relation - relation.reorder! other.order_values + relation.reorder!(*other.order_values) elsif other.order_values.any? # merge in order_values from relation - relation.order! other.order_values + relation.order!(*other.order_values) end extensions = other.extensions - relation.extensions diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index f53ef1fe35..6e7998d15a 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -121,6 +121,16 @@ class RelationMergingTest < ActiveRecord::TestCase relation = relation.merge(Post.from("posts")) assert_not_empty relation.from_clause end + + def test_merging_with_order_with_binds + relation = Post.all.merge(Post.order([Arel.sql("title LIKE ?"), "%suffix"])) + assert_equal ["title LIKE '%suffix'"], relation.order_values + end + + def test_merging_with_order_without_binds + relation = Post.all.merge(Post.order(Arel.sql("title LIKE '%?'"))) + assert_equal ["title LIKE '%?'"], relation.order_values + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase |