mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Unify access to bind values on Relation
The bind values can come from four places. `having`, `where`, `joins`, and `from` when selecting from a subquery that contains binds. These need to be kept in a specific order, since the clauses will always appear in that order. Up until recently, they were not. Additionally, `joins` actually did keep its bind values in a separate location (presumably because it's the only case that people noticed was broken). However, this meant that anything accessing just `bind_values` was broken (which most places were). This is no longer possible, there is only a single way to access the bind values, and it includes joins in the proper location. The setter was removed yesterday, so breaking `+=` cases is not possible. I'm still not happy that `joins` is putting it's bind values on the Arel AST, and I'm planning on refactoring it further, but this removes a ton of bug cases.
This commit is contained in:
parent
ad20b41cb9
commit
16ce2eecd3
6 changed files with 13 additions and 21 deletions
|
@ -342,8 +342,7 @@ module ActiveRecord
|
|||
stmt.wheres = arel.constraints
|
||||
end
|
||||
|
||||
bvs = arel.bind_values + bind_values
|
||||
@klass.connection.update stmt, 'SQL', bvs
|
||||
@klass.connection.update stmt, 'SQL', bind_values
|
||||
end
|
||||
|
||||
# Updates an object (or multiple objects) and saves it to the database, if validations pass.
|
||||
|
@ -559,11 +558,10 @@ module ActiveRecord
|
|||
find_with_associations { |rel| relation = rel }
|
||||
end
|
||||
|
||||
arel = relation.arel
|
||||
binds = arel.bind_values + relation.bind_values
|
||||
binds = relation.bind_values
|
||||
binds = connection.prepare_binds_for_database(binds)
|
||||
binds.map! { |_, value| connection.quote(value) }
|
||||
collect = visitor.accept(arel.ast, Arel::Collectors::Bind.new)
|
||||
collect = visitor.accept(relation.arel.ast, Arel::Collectors::Bind.new)
|
||||
collect.substitute_binds(binds).join
|
||||
end
|
||||
end
|
||||
|
@ -636,7 +634,7 @@ module ActiveRecord
|
|||
private
|
||||
|
||||
def exec_queries
|
||||
@records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, arel.bind_values + bind_values)
|
||||
@records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
|
||||
|
||||
preload = preload_values
|
||||
preload += includes_values unless eager_loading?
|
||||
|
|
|
@ -166,7 +166,7 @@ module ActiveRecord
|
|||
relation.select_values = column_names.map { |cn|
|
||||
columns_hash.key?(cn) ? arel_table[cn] : cn
|
||||
}
|
||||
result = klass.connection.select_all(relation.arel, nil, relation.arel.bind_values + bind_values)
|
||||
result = klass.connection.select_all(relation.arel, nil, bind_values)
|
||||
result.cast_values(klass.column_types)
|
||||
end
|
||||
end
|
||||
|
@ -227,14 +227,11 @@ module ActiveRecord
|
|||
|
||||
column_alias = column_name
|
||||
|
||||
bind_values = nil
|
||||
|
||||
if operation == "count" && (relation.limit_value || relation.offset_value)
|
||||
# Shortcut when limit is zero.
|
||||
return 0 if relation.limit_value == 0
|
||||
|
||||
query_builder = build_count_subquery(relation, column_name, distinct)
|
||||
bind_values = query_builder.bind_values + relation.bind_values
|
||||
else
|
||||
column = aggregate_column(column_name)
|
||||
|
||||
|
@ -245,7 +242,6 @@ module ActiveRecord
|
|||
relation.select_values = [select_value]
|
||||
|
||||
query_builder = relation.arel
|
||||
bind_values = query_builder.bind_values + relation.bind_values
|
||||
end
|
||||
|
||||
result = @klass.connection.select_all(query_builder, nil, bind_values)
|
||||
|
@ -304,7 +300,7 @@ module ActiveRecord
|
|||
relation.group_values = group
|
||||
relation.select_values = select_values
|
||||
|
||||
calculated_data = @klass.connection.select_all(relation, nil, relation.arel.bind_values + bind_values)
|
||||
calculated_data = @klass.connection.select_all(relation, nil, relation.bind_values)
|
||||
|
||||
if association
|
||||
key_ids = calculated_data.collect { |row| row[group_aliases.first] }
|
||||
|
@ -378,11 +374,9 @@ module ActiveRecord
|
|||
|
||||
aliased_column = aggregate_column(column_name == :all ? 1 : column_name).as(column_alias)
|
||||
relation.select_values = [aliased_column]
|
||||
arel = relation.arel
|
||||
subquery = arel.as(subquery_alias)
|
||||
subquery = relation.arel.as(subquery_alias)
|
||||
|
||||
sm = Arel::SelectManager.new relation.engine
|
||||
sm.bind_values = arel.bind_values
|
||||
select_value = operation_over_aggregate_column(column_alias, 'count', distinct)
|
||||
sm.project(select_value).from(subquery)
|
||||
end
|
||||
|
|
|
@ -311,7 +311,7 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
connection.select_value(relation, "#{name} Exists", relation.arel.bind_values + relation.bind_values) ? true : false
|
||||
connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false
|
||||
end
|
||||
|
||||
# This method is called whenever no records are found with either a single
|
||||
|
@ -365,7 +365,7 @@ module ActiveRecord
|
|||
[]
|
||||
else
|
||||
arel = relation.arel
|
||||
rows = connection.select_all(arel, 'SQL', arel.bind_values + relation.bind_values)
|
||||
rows = connection.select_all(arel, 'SQL', relation.bind_values)
|
||||
join_dependency.instantiate(rows, aliases)
|
||||
end
|
||||
end
|
||||
|
@ -410,7 +410,7 @@ module ActiveRecord
|
|||
relation = relation.except(:select).select(values).distinct!
|
||||
arel = relation.arel
|
||||
|
||||
id_rows = @klass.connection.select_all(arel, 'SQL', arel.bind_values + relation.bind_values)
|
||||
id_rows = @klass.connection.select_all(arel, 'SQL', relation.bind_values)
|
||||
id_rows.map {|row| row[primary_key]}
|
||||
end
|
||||
|
||||
|
|
|
@ -10,7 +10,7 @@ module ActiveRecord
|
|||
|
||||
def binds
|
||||
if value.is_a?(Relation)
|
||||
value.arel.bind_values + value.bind_values
|
||||
value.bind_values
|
||||
else
|
||||
[]
|
||||
end
|
||||
|
|
|
@ -104,7 +104,7 @@ module ActiveRecord
|
|||
result[column_name] = attrs
|
||||
binds += bvs
|
||||
when Relation
|
||||
binds += value.arel.bind_values + value.bind_values
|
||||
binds += value.bind_values
|
||||
else
|
||||
if can_be_bound?(column_name, value)
|
||||
result[column_name] = Arel::Nodes::BindParam.new
|
||||
|
|
|
@ -94,7 +94,7 @@ module ActiveRecord
|
|||
end
|
||||
|
||||
def bind_values
|
||||
from_clause.binds + where_clause.binds + having_clause.binds
|
||||
from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds
|
||||
end
|
||||
|
||||
def create_with_value # :nodoc:
|
||||
|
|
Loading…
Reference in a new issue