Merge pull request #12588 from jetthoughts/12586_subquery_with_unprepared_sql

Inline bind values for sub-queries generated for Relation in where

Conflicts:
	activerecord/CHANGELOG.md

activerecord/CHANGELOG.md

@@ -1,3 +1,12 @@
+*    Sub-query generated for `Relation` passed as array condition did not take in account
+     bind values and have invalid syntax.
+
+     Generate sub-query with inline bind values.
+
+     Fixes: #12586
+
+     *Paul Nikitochkin*
+
 *   Fix a bug where rake db:structure:load crashed when the path contained
     spaces.
 

activerecord/lib/active_record/relation/query_methods.rb

@@ -894,6 +894,13 @@ def collapse_wheres(arel, wheres)
     def build_where(opts, other = [])
       case opts
       when String, Array
+        #TODO: Remove duplication with: /activerecord/lib/active_record/sanitization.rb:113
+        values = Hash === other.first ? other.first.values : other
+
+        values.grep(ActiveRecord::Relation) do |rel|
+          self.bind_values += rel.bind_values
+        end
+
         [@klass.send(:sanitize_sql, other.empty? ? opts : ([opts] + other))]
       when Hash
         opts = PredicateBuilder.resolve_column_aliases(klass, opts)

activerecord/test/cases/relations_test.rb

@@ -639,6 +639,31 @@ def test_find_all_using_where_with_relation
       relation = Author.where('id in (?)', Author.where(id: david).select(:id))
       assert_equal [david], relation.to_a
     }
+
+    assert_queries(1) do
+      relation = Author.where('id in (:author_ids)', author_ids: Author.where(id: david).select(:id))
+      assert_equal [david], relation.to_a
+    end
+  end
+
+  def test_find_all_using_where_with_relation_with_bound_values
+    david = authors(:david)
+    davids_posts = david.posts.to_a
+
+    assert_queries(1) do
+      relation = Post.where(id: david.posts.select(:id))
+      assert_equal davids_posts, relation.to_a
+    end
+
+    assert_queries(1) do
+      relation = Post.where('id in (?)', david.posts.select(:id))
+      assert_equal davids_posts, relation.to_a, 'should process Relation as bind variables'
+    end
+
+    assert_queries(1) do
+      relation = Post.where('id in (:post_ids)', post_ids: david.posts.select(:id))
+      assert_equal davids_posts, relation.to_a, 'should process Relation as named bind variables'
+    end
   end
 
   def test_find_all_using_where_with_relation_and_alternate_primary_key

activerecord/test/cases/sanitize_test.rb

@@ -1,5 +1,7 @@
 require "cases/helper"
 require 'models/binary'
+require 'models/author'
+require 'models/post'
 
 class SanitizeTest < ActiveRecord::TestCase
   def setup
@@ -33,8 +35,15 @@ def test_sanitize_sql_array_handles_bind_variables
   end
 
   def test_sanitize_sql_array_handles_relations
-    assert_match(/\(\bselect\b.*?\bwhere\b.*?\)/i,
-      Binary.send(:sanitize_sql_array, ["id in (?)", Binary.where(id: 1)]),
-      "should sanitize `Relation` as subquery")
+    david = Author.create!(name: 'David')
+    david_posts = david.posts.select(:id)
+
+    sub_query_pattern = /\(\bselect\b.*?\bwhere\b.*?\)/i
+
+    select_author_sql = Post.send(:sanitize_sql_array, ['id in (?)', david_posts])
+    assert_match(sub_query_pattern, select_author_sql, 'should sanitize `Relation` as subquery for bind variables')
+
+    select_author_sql = Post.send(:sanitize_sql_array, ['id in (:post_ids)', post_ids: david_posts])
+    assert_match(sub_query_pattern, select_author_sql, 'should sanitize `Relation` as subquery for named bind variables')
   end
 end