Skip to content

Commit c99bea1

Browse files
committed
Use bind params for values other than strings and ints
Booleans, for example, are a perfectly valid value to use for bind params. Required to allow rails#19874 to be fixed. There is an unrelated looking change in `through_association` also required for this fix. We have a polymorphic association in our tests that is unscoping a boolean query parameter. Mutating the association as we were doing previously causes too many bind params to be on the final query.
1 parent 44725d9 commit c99bea1

File tree

4 files changed

+33
-8
lines changed

4 files changed

+33
-8
lines changed

activerecord/lib/active_record/associations/through_association.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def target_scope
1818

1919
reflection_scope = reflection.scope
2020
if reflection_scope && reflection_scope.arity.zero?
21-
relation.merge!(reflection_scope)
21+
relation = relation.merge(reflection_scope)
2222
end
2323

2424
scope.merge!(

activerecord/lib/active_record/relation/predicate_builder.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,8 @@ def self.register_handler(klass, handler)
112112
@handlers.unshift([klass, handler])
113113
end
114114

115-
register_handler(BasicObject, ->(attribute, value) { attribute.eq(value) })
115+
BASIC_OBJECT_HANDLER = ->(attribute, value) { attribute.eq(value) } # :nodoc:
116+
register_handler(BasicObject, BASIC_OBJECT_HANDLER)
116117
# FIXME: I think we need to deprecate this behavior
117118
register_handler(Class, ->(attribute, value) { attribute.eq(value.name) })
118119
register_handler(Base, ->(attribute, value) { attribute.eq(value.id) })
@@ -142,5 +143,11 @@ def self.convert_value_to_association_ids(value, primary_key)
142143
value
143144
end
144145
end
146+
147+
def self.can_be_bound?(value) # :nodoc:
148+
!value.nil? &&
149+
!value.is_a?(Hash) &&
150+
handler_for(value) == BASIC_OBJECT_HANDLER
151+
end
145152
end
146153
end

activerecord/lib/active_record/relation/query_methods.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -965,12 +965,9 @@ def build_where(opts, other = [])
965965

966966
def create_binds(opts)
967967
bindable, non_binds = opts.partition do |column, value|
968-
case value
969-
when String, Integer, ActiveRecord::StatementCache::Substitute
970-
@klass.columns_hash.include? column.to_s
971-
else
972-
false
973-
end
968+
PredicateBuilder.can_be_bound?(value) &&
969+
@klass.columns_hash.include?(column.to_s) &&
970+
!@klass.reflect_on_aggregation(column)
974971
end
975972

976973
association_binds, non_binds = non_binds.partition do |column, value|

activerecord/test/cases/attributes_test.rb

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,5 +111,26 @@ class CustomPropertiesTest < ActiveRecord::TestCase
111111
assert klass.column_names.include?('wibble')
112112
assert_equal 6, klass.content_columns.length
113113
end
114+
115+
test "non string/integers use custom types for queries" do
116+
klass = Class.new(OverloadedType)
117+
type = Type::Value.new
118+
def type.cast_value(value)
119+
!!value
120+
end
121+
122+
def type.type_cast_for_database(value)
123+
if value
124+
"Y"
125+
else
126+
"N"
127+
end
128+
end
129+
130+
klass.attribute(:string_with_default, type, default: false)
131+
klass.create!(string_with_default: true)
132+
133+
assert_equal 1, klass.where(string_with_default: true).count
134+
end
114135
end
115136
end

0 commit comments

Comments
 (0)