mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Don't rely on relation mutability when building through associations
Specifically, the issue is relying on `where_unscoping` mutating the where values. It does not, however, mutate the bind values, which could cause an error under certain circumstances. This was not exposed by the tests, since the only place which would have been affected is unscoping a boolean, which doesn't go through prepared statements. I had a hard time getting better test coverage to demonstrate the issue. This in turn, caused `merge` to go through proper logic, and try to clear out the binds associated with the unscoped relation, which then exposed a source of `nil` for the columns, as binds weren't expanding `{ "posts.id" => 1 }` to `{ "posts" => { "id" => 1 } }`. This has been fixed. The bulk of `create_binds` needed to be moved to a separate method, since the dot notation should not be expanded recursively. I'm pretty sure this removes a subtle quirk that a ton of code in `Relation::Merger` is working around, and I suspect that code can be greatly simplified. However, unraveling that rats nest is no small task.
This commit is contained in:
parent
4c0a9922d6
commit
c80487eb1a
2 changed files with 25 additions and 19 deletions
|
@ -18,7 +18,7 @@ module ActiveRecord
|
|||
|
||||
reflection_scope = reflection.scope
|
||||
if reflection_scope && reflection_scope.arity.zero?
|
||||
relation.merge!(reflection_scope)
|
||||
relation = relation.merge(reflection_scope)
|
||||
end
|
||||
|
||||
scope.merge!(
|
||||
|
|
|
@ -29,24 +29,8 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def create_binds(attributes)
|
||||
result = attributes.dup
|
||||
binds = []
|
||||
|
||||
attributes.each do |column_name, value|
|
||||
case value
|
||||
when String, Integer, ActiveRecord::StatementCache::Substitute
|
||||
result[column_name] = Arel::Nodes::BindParam.new
|
||||
binds.push([table.column(column_name), value])
|
||||
when Hash
|
||||
attrs, bvs = associated_predicate_builder(column_name).create_binds(value)
|
||||
result[column_name] = attrs
|
||||
binds += bvs
|
||||
when Relation
|
||||
binds += value.arel.bind_values + value.bind_values
|
||||
end
|
||||
end
|
||||
|
||||
[result, binds]
|
||||
attributes = convert_dot_notation_to_hash(attributes.stringify_keys)
|
||||
create_binds_for_hash(attributes)
|
||||
end
|
||||
|
||||
def expand(column, value)
|
||||
|
@ -108,6 +92,28 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
|
||||
def create_binds_for_hash(attributes)
|
||||
result = attributes.dup
|
||||
binds = []
|
||||
|
||||
attributes.each do |column_name, value|
|
||||
case value
|
||||
when String, Integer, ActiveRecord::StatementCache::Substitute
|
||||
result[column_name] = Arel::Nodes::BindParam.new
|
||||
binds.push([table.column(column_name), value])
|
||||
when Hash
|
||||
attrs, bvs = associated_predicate_builder(column_name).create_binds_for_hash(value)
|
||||
result[column_name] = attrs
|
||||
binds += bvs
|
||||
when Relation
|
||||
binds += value.arel.bind_values + value.bind_values
|
||||
end
|
||||
end
|
||||
|
||||
[result, binds]
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def associated_predicate_builder(association_name)
|
||||
|
|
Loading…
Reference in a new issue