aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorPaul Nikitochkin <paul.nikitochkin@gmail.com>2013-10-18 23:26:39 +0300
committerPaul Nikitochkin <paul.nikitochkin@gmail.com>2013-10-20 21:07:07 +0300
commita2ed5d2381079716cf6224bc7b0538d318b264a2 (patch)
tree782bc3765e915d91651402d3f079825ba7f048dc /activerecord
parentdb82e1d0bb21c2a13354b97733c09737719e946b (diff)
downloadrails-a2ed5d2381079716cf6224bc7b0538d318b264a2.tar.gz
rails-a2ed5d2381079716cf6224bc7b0538d318b264a2.tar.bz2
rails-a2ed5d2381079716cf6224bc7b0538d318b264a2.zip
Process sub-query relation's binding values
Generated sub-query for Relation as array condition for `where` method did not take in account its bind values, in result generates invalid SQL query. Fixed by adding sub-query relation's binding values to base relation Closes: #12586
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md9
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb7
-rw-r--r--activerecord/test/cases/relations_test.rb25
-rw-r--r--activerecord/test/cases/sanitize_test.rb17
4 files changed, 54 insertions, 4 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 815e7c71fe..0cb52614eb 100644
--- a/activerecord/CHANGELOG.md
+++ b/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*
+
* `rake db:structure:dump` no longer crashes when the port was specified as `Fixnum`.
*Kenta Okamoto*
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 9c9690215a..f680d94215 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -894,6 +894,13 @@ module ActiveRecord
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)
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 860bd424b7..55501ea35b 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -639,6 +639,31 @@ class RelationTest < ActiveRecord::TestCase
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
diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb
index 4c0762deca..766b2ff2ef 100644
--- a/activerecord/test/cases/sanitize_test.rb
+++ b/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
@@ -9,7 +11,7 @@ class SanitizeTest < ActiveRecord::TestCase
quoted_bambi = ActiveRecord::Base.connection.quote("Bambi")
quoted_column_name = ActiveRecord::Base.connection.quote_column_name("name")
quoted_table_name = ActiveRecord::Base.connection.quote_table_name("adorable_animals")
- expected_value = "#{quoted_table_name}.#{quoted_column_name} = #{quoted_bambi}"
+ expected_value = "#{quoted_table_name}.#{quoted_column_name} = #{quoted_bambi}"
assert_equal expected_value, Binary.send(:sanitize_sql_hash, {adorable_animals: {name: 'Bambi'}})
end
@@ -33,8 +35,15 @@ class SanitizeTest < ActiveRecord::TestCase
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