aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2017-05-11 04:20:53 +0900
committerRyuta Kamizono <kamipo@gmail.com>2017-05-31 17:16:11 +0900
commit67a4a9feb9d31747db8a9ce5fcfe61d6067dd625 (patch)
treef9617483d55b26143da444205509173241b605b0
parent7c800fe5e338540ef195f071a7f3604ed4197f3f (diff)
downloadrails-67a4a9feb9d31747db8a9ce5fcfe61d6067dd625.tar.gz
rails-67a4a9feb9d31747db8a9ce5fcfe61d6067dd625.tar.bz2
rails-67a4a9feb9d31747db8a9ce5fcfe61d6067dd625.zip
Prevent making bind param if casted value is nil
If casted value is nil, generated SQL should be `IS NULL`. But currently it is generated as `= NULL`. To prevent this behavior, avoid making bind param if casted value is nil. Fixes #28945.
-rw-r--r--activerecord/CHANGELOG.md4
-rw-r--r--activerecord/lib/active_record/relation/predicate_builder.rb19
-rw-r--r--activerecord/lib/active_record/relation/where_clause_factory.rb2
-rw-r--r--activerecord/test/cases/enum_test.rb2
-rw-r--r--activerecord/test/cases/relation/where_test.rb6
5 files changed, 24 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 1f8163db12..f75f1a9108 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+* Prevent making bind param if casted value is nil.
+
+ *Ryuta Kamizono*
+
* Deprecate passing arguments and block at the same time to `count` and `sum` in `ActiveRecord::Calculations`.
*Ryuta Kamizono*
diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb
index a6309e0b5c..7dea5deec5 100644
--- a/activerecord/lib/active_record/relation/predicate_builder.rb
+++ b/activerecord/lib/active_record/relation/predicate_builder.rb
@@ -107,21 +107,26 @@ module ActiveRecord
first = value.begin
last = value.end
unless first.respond_to?(:infinite?) && first.infinite?
- binds << build_bind_param(column_name, first)
+ binds << build_bind_attribute(column_name, first)
first = Arel::Nodes::BindParam.new
end
unless last.respond_to?(:infinite?) && last.infinite?
- binds << build_bind_param(column_name, last)
+ binds << build_bind_attribute(column_name, last)
last = Arel::Nodes::BindParam.new
end
result[column_name] = RangeHandler::RangeWithBinds.new(first, last, value.exclude_end?)
+ when value.is_a?(Relation)
+ binds.concat(value.bound_attributes)
else
if can_be_bound?(column_name, value)
- result[column_name] = Arel::Nodes::BindParam.new
- binds << build_bind_param(column_name, value)
- elsif value.is_a?(Relation)
- binds.concat(value.bound_attributes)
+ bind_attribute = build_bind_attribute(column_name, value)
+ if value.is_a?(StatementCache::Substitute) || !bind_attribute.value_for_database.nil?
+ result[column_name] = Arel::Nodes::BindParam.new
+ binds << bind_attribute
+ else
+ result[column_name] = nil
+ end
end
end
end
@@ -164,7 +169,7 @@ module ActiveRecord
end
end
- def build_bind_param(column_name, value)
+ def build_bind_attribute(column_name, value)
Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name))
end
end
diff --git a/activerecord/lib/active_record/relation/where_clause_factory.rb b/activerecord/lib/active_record/relation/where_clause_factory.rb
index 04bee73e8f..b862dd56a5 100644
--- a/activerecord/lib/active_record/relation/where_clause_factory.rb
+++ b/activerecord/lib/active_record/relation/where_clause_factory.rb
@@ -57,7 +57,7 @@ module ActiveRecord
else
column = klass.column_for_attribute(attribute)
- binds << predicate_builder.send(:build_bind_param, attribute, value)
+ binds << predicate_builder.send(:build_bind_attribute, attribute, value)
value = Arel::Nodes::BindParam.new
predicate = if options[:case_sensitive]
diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb
index db3da53487..4ef9a125e6 100644
--- a/activerecord/test/cases/enum_test.rb
+++ b/activerecord/test/cases/enum_test.rb
@@ -60,6 +60,7 @@ class EnumTest < ActiveRecord::TestCase
assert_not_equal @book, Book.where(status: [:written]).first
assert_not_equal @book, Book.where.not(status: :published).first
assert_equal @book, Book.where.not(status: :written).first
+ assert_equal books(:ddd), Book.where(read_status: :forgotten).first
end
test "find via where with strings" do
@@ -69,6 +70,7 @@ class EnumTest < ActiveRecord::TestCase
assert_not_equal @book, Book.where(status: ["written"]).first
assert_not_equal @book, Book.where.not(status: "published").first
assert_equal @book, Book.where.not(status: "written").first
+ assert_equal books(:ddd), Book.where(read_status: "forgotten").first
end
test "build from scope" do
diff --git a/activerecord/test/cases/relation/where_test.rb b/activerecord/test/cases/relation/where_test.rb
index cbc466d6b8..42dae4d569 100644
--- a/activerecord/test/cases/relation/where_test.rb
+++ b/activerecord/test/cases/relation/where_test.rb
@@ -15,7 +15,7 @@ require "models/vertex"
module ActiveRecord
class WhereTest < ActiveRecord::TestCase
- fixtures :posts, :edges, :authors, :author_addresses, :binaries, :essays, :cars, :treasures, :price_estimates
+ fixtures :posts, :edges, :authors, :author_addresses, :binaries, :essays, :cars, :treasures, :price_estimates, :topics
def test_where_copies_bind_params
author = authors(:david)
@@ -48,6 +48,10 @@ module ActiveRecord
assert_equal [chef], chefs.to_a
end
+ def test_where_with_casted_value_is_nil
+ assert_equal 4, Topic.where(last_read: "").count
+ end
+
def test_rewhere_on_root
assert_equal posts(:welcome), Post.rewhere(title: "Welcome to the weblog").first
end