From 96cd16bdeec661c9ecf1a83ca41a2cb22f435af9 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Mon, 20 Aug 2018 23:41:45 +0900
Subject: 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.
---
 activerecord/lib/active_record/relation/merger.rb |  4 ++--
 activerecord/test/cases/relation/merging_test.rb  | 10 ++++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

(limited to 'activerecord')

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
-- 
cgit v1.2.3