Skip to content

Commit

Permalink
Remove Relation#bind_params
Browse files Browse the repository at this point in the history
`bound_attributes` is now used universally across the board, removing
the need for the conversion layer. These changes are mostly mechanical,
with the exception of the log subscriber. Additional, we had to
implement `hash` on the attribute objects, so they could be used as a
key for query caching.
  • Loading branch information
sgrif committed Jan 27, 2015
1 parent d66ffb6 commit b06f64c
Show file tree
Hide file tree
Showing 30 changed files with 116 additions and 130 deletions.
Expand Up @@ -143,7 +143,7 @@ def delete_records(records, method)
stmt.from scope.klass.arel_table
stmt.wheres = arel.constraints

count = scope.klass.connection.delete(stmt, 'SQL', scope.bind_values)
count = scope.klass.connection.delete(stmt, 'SQL', scope.bound_attributes)
end
when :nullify
count = scope.update_all(source_reflection.foreign_key => nil)
Expand Down
Expand Up @@ -75,7 +75,7 @@ def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scop
column = klass.columns_hash[reflection.type.to_s]

substitute = klass.connection.substitute_at(column)
binds << Attribute.with_cast_value(column.name, value, klass.type_for_attribute(column.name))
binds << Relation::QueryAttribute.new(column.name, value, klass.type_for_attribute(column.name))
constraint = constraint.and table[reflection.type].eq substitute
end

Expand Down
5 changes: 5 additions & 0 deletions activerecord/lib/active_record/attribute.rb
Expand Up @@ -88,6 +88,11 @@ def ==(other)
value_before_type_cast == other.value_before_type_cast &&
type == other.type
end
alias eql? ==

def hash
[self.class, name, value_before_type_cast, type].hash
end

protected

Expand Down
Expand Up @@ -286,10 +286,10 @@ def insert_fixture(fixture, table_name)
columns = schema_cache.columns_hash(table_name)

binds = fixture.map do |name, value|
[columns[name], value]
Relation::QueryAttribute.new(name, value, columns[name].cast_type)
end
key_list = fixture.keys.map { |name| quote_column_name(name) }
value_list = prepare_binds_for_database(binds).map do |_, value|
value_list = prepare_binds_for_database(binds).map do |value|
begin
quote(value)
rescue TypeError
Expand Down Expand Up @@ -381,7 +381,7 @@ def last_inserted_id(result)

def binds_from_relation(relation, binds)
if relation.is_a?(Relation) && binds.empty?
relation, binds = relation.arel, relation.bind_values
relation, binds = relation.arel, relation.bound_attributes
end
[relation, binds]
end
Expand Down
Expand Up @@ -97,13 +97,7 @@ def quoted_date(value)
end

def prepare_binds_for_database(binds) # :nodoc:
binds.map do |column, value|
if column
column_name = column.name
value = column.cast_type.type_cast_for_database(value)
end
[column_name, value]
end
binds.map(&:value_for_database)
end

private
Expand Down
Expand Up @@ -114,7 +114,7 @@ def initialize(connection, logger = nil, pool = nil) #:nodoc:
class BindCollector < Arel::Collectors::Bind
def compile(bvs, conn)
casted_binds = conn.prepare_binds_for_database(bvs)
super(casted_binds.map { |_, value| conn.quote(value) })
super(casted_binds.map { |value| conn.quote(value) })
end
end

Expand Down
Expand Up @@ -395,11 +395,9 @@ def begin_db_transaction #:nodoc:

def exec_stmt(sql, name, binds)
cache = {}
type_casted_binds = binds.map { |col, val|
[col, type_cast(val, col)]
}
type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) }

log(sql, name, type_casted_binds) do
log(sql, name, binds) do
if binds.empty?
stmt = @connection.prepare(sql)
else
Expand All @@ -410,7 +408,7 @@ def exec_stmt(sql, name, binds)
end

begin
stmt.execute(*type_casted_binds.map { |_, val| val })
stmt.execute(*type_casted_binds)
rescue Mysql::Error => e
# Older versions of MySQL leave the prepared statement in a bad
# place when an error occurs. To support older MySQL versions, we
Expand Down
Expand Up @@ -609,12 +609,10 @@ def exec_no_cache(sql, name, binds)

def exec_cache(sql, name, binds)
stmt_key = prepare_statement(sql)
type_casted_binds = binds.map { |col, val|
[col, type_cast(val, col)]
}
type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) }

log(sql, name, type_casted_binds, stmt_key) do
@connection.exec_prepared(stmt_key, type_casted_binds.map { |_, val| val })
log(sql, name, binds, stmt_key) do
@connection.exec_prepared(stmt_key, type_casted_binds)
end
rescue ActiveRecord::StatementInvalid => e
pgerror = e.original_exception
Expand Down
Expand Up @@ -280,11 +280,9 @@ def pp(result) # :nodoc:
end

def exec_query(sql, name = nil, binds = [])
type_casted_binds = binds.map { |col, val|
[col, type_cast(val, col)]
}
type_casted_binds = binds.map { |attr| type_cast(attr.value_for_database) }

log(sql, name, type_casted_binds) do
log(sql, name, binds) do
# Don't cache statements if they are not prepared
if without_prepared_statement?(binds)
stmt = @connection.prepare(sql)
Expand All @@ -302,7 +300,7 @@ def exec_query(sql, name = nil, binds = [])
stmt = cache[:stmt]
cols = cache[:cols] ||= stmt.columns
stmt.reset!
stmt.bind_params type_casted_binds.map { |_, val| val }
stmt.bind_params type_casted_binds
end

ActiveRecord::Result.new(cols, stmt.to_a)
Expand Down
20 changes: 7 additions & 13 deletions activerecord/lib/active_record/log_subscriber.rb
Expand Up @@ -20,18 +20,14 @@ def initialize
@odd = false
end

def render_bind(column, value)
if column
if column.binary? && value
# This specifically deals with the PG adapter that casts bytea columns into a Hash.
value = value[:value] if value.is_a?(Hash)
value = "<#{value.bytesize} bytes of binary data>"
end

[column.name, value]
def render_bind(attribute)
value = if attribute.type.binary? && attribute.value
"<#{attribute.value.bytesize} bytes of binary data>"
else
[nil, value]
attribute.value_for_database
end

[attribute.name, value]
end

def sql(event)
Expand All @@ -47,9 +43,7 @@ def sql(event)
binds = nil

unless (payload[:binds] || []).empty?
binds = " " + payload[:binds].map { |col,v|
render_bind(col, v)
}.inspect
binds = " " + payload[:binds].map { |attr| render_bind(attr) }.inspect
end

if odd?
Expand Down
18 changes: 9 additions & 9 deletions activerecord/lib/active_record/relation.rb
Expand Up @@ -81,7 +81,7 @@ def _update_record(values, id, id_was) # :nodoc:
end

relation = scope.where(@klass.primary_key => (id_was || id))
bvs = binds + relation.bind_values
bvs = binds + relation.bound_attributes
um = relation
.arel
.compile_update(substitutes, @klass.primary_key)
Expand All @@ -95,11 +95,11 @@ def _update_record(values, id, id_was) # :nodoc:

def substitute_values(values) # :nodoc:
binds = values.map do |arel_attr, value|
[@klass.columns_hash[arel_attr.name], value]
QueryAttribute.new(arel_attr.name, value, klass.type_for_attribute(arel_attr.name))
end

substitutes = values.each_with_index.map do |(arel_attr, _), i|
[arel_attr, @klass.connection.substitute_at(binds[i][0])]
substitutes = values.map do |(arel_attr, _)|
[arel_attr, connection.substitute_at(klass.columns_hash[arel_attr.name])]
end

[substitutes, binds]
Expand Down Expand Up @@ -342,7 +342,7 @@ def update_all(updates)
stmt.wheres = arel.constraints
end

@klass.connection.update stmt, 'SQL', bind_values
@klass.connection.update stmt, 'SQL', bound_attributes
end

# Updates an object (or multiple objects) and saves it to the database, if validations pass.
Expand Down Expand Up @@ -488,7 +488,7 @@ def delete_all(conditions = nil)
stmt.wheres = arel.constraints
end

affected = @klass.connection.delete(stmt, 'SQL', bind_values)
affected = @klass.connection.delete(stmt, 'SQL', bound_attributes)

reset
affected
Expand Down Expand Up @@ -558,9 +558,9 @@ def to_sql
find_with_associations { |rel| relation = rel }
end

binds = relation.bind_values
binds = relation.bound_attributes
binds = connection.prepare_binds_for_database(binds)
binds.map! { |_, value| connection.quote(value) }
binds.map! { |value| connection.quote(value) }
collect = visitor.accept(relation.arel.ast, Arel::Collectors::Bind.new)
collect.substitute_binds(binds).join
end
Expand Down Expand Up @@ -634,7 +634,7 @@ def inspect
private

def exec_queries
@records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bind_values)
@records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel, bound_attributes)

preload = preload_values
preload += includes_values unless eager_loading?
Expand Down
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/calculations.rb
Expand Up @@ -166,7 +166,7 @@ def pluck(*column_names)
relation.select_values = column_names.map { |cn|
columns_hash.key?(cn) ? arel_table[cn] : cn
}
result = klass.connection.select_all(relation.arel, nil, bind_values)
result = klass.connection.select_all(relation.arel, nil, bound_attributes)
result.cast_values(klass.column_types)
end
end
Expand Down Expand Up @@ -244,7 +244,7 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
query_builder = relation.arel
end

result = @klass.connection.select_all(query_builder, nil, bind_values)
result = @klass.connection.select_all(query_builder, nil, bound_attributes)
row = result.first
value = row && row.values.first
column = result.column_types.fetch(column_alias) do
Expand Down Expand Up @@ -300,7 +300,7 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
relation.group_values = group
relation.select_values = select_values

calculated_data = @klass.connection.select_all(relation, nil, relation.bind_values)
calculated_data = @klass.connection.select_all(relation, nil, relation.bound_attributes)

if association
key_ids = calculated_data.collect { |row| row[group_aliases.first] }
Expand Down
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/finder_methods.rb
Expand Up @@ -311,7 +311,7 @@ def exists?(conditions = :none)
end
end

connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false
connection.select_value(relation, "#{name} Exists", relation.bound_attributes) ? true : false
end

# This method is called whenever no records are found with either a single
Expand Down Expand Up @@ -365,7 +365,7 @@ def find_with_associations
[]
else
arel = relation.arel
rows = connection.select_all(arel, 'SQL', relation.bind_values)
rows = connection.select_all(arel, 'SQL', relation.bound_attributes)
join_dependency.instantiate(rows, aliases)
end
end
Expand Down Expand Up @@ -410,7 +410,7 @@ def limited_ids_for(relation)
relation = relation.except(:select).select(values).distinct!
arel = relation.arel

id_rows = @klass.connection.select_all(arel, 'SQL', relation.bind_values)
id_rows = @klass.connection.select_all(arel, 'SQL', relation.bound_attributes)
id_rows.map {|row| row[primary_key]}
end

Expand Down
Expand Up @@ -108,7 +108,7 @@ def create_binds_for_hash(attributes)
else
if can_be_bound?(column_name, value)
result[column_name] = Arel::Nodes::BindParam.new
binds << Attribute.with_cast_value(column_name.to_s, value, table.type(column_name))
binds << Relation::QueryAttribute.new(column_name.to_s, value, table.type(column_name))
end
end
end
Expand Down
19 changes: 19 additions & 0 deletions activerecord/lib/active_record/relation/query_attribute.rb
@@ -0,0 +1,19 @@
require 'active_record/attribute'

module ActiveRecord
class Relation
class QueryAttribute < Attribute
def type_cast(value)
value
end

def value_for_database
@value_for_database ||= super
end

def with_cast_value(value)
QueryAttribute.new(name, value, type)
end
end
end
end
11 changes: 1 addition & 10 deletions activerecord/lib/active_record/relation/query_methods.rb
@@ -1,4 +1,5 @@
require "active_record/relation/from_clause"
require "active_record/relation/query_attribute"
require "active_record/relation/where_clause"
require "active_record/relation/where_clause_factory"
require 'active_model/forbidden_attributes_protection'
Expand Down Expand Up @@ -96,16 +97,6 @@ def bound_attributes
from_clause.binds + arel.bind_values + where_clause.binds + having_clause.binds
end

def bind_values
# convert to old style
bound_attributes.map do |attribute|
if attribute.name
column = ConnectionAdapters::Column.new(attribute.name, nil, attribute.type)
end
[column, attribute.value_before_type_cast]
end
end

def create_with_value # :nodoc:
@values[:create_with] || {}
end
Expand Down
18 changes: 9 additions & 9 deletions activerecord/lib/active_record/statement_cache.rb
Expand Up @@ -48,7 +48,7 @@ def initialize values
def sql_for(binds, connection)
val = @values.dup
binds = connection.prepare_binds_for_database(binds)
@indexes.each { |i| val[i] = connection.quote(binds.shift.last) }
@indexes.each { |i| val[i] = connection.quote(binds.shift) }
val.join
end
end
Expand All @@ -67,29 +67,29 @@ def bind; Substitute.new; end
end

class BindMap # :nodoc:
def initialize(bind_values)
def initialize(bound_attributes)
@indexes = []
@bind_values = bind_values
@bound_attributes = bound_attributes

bind_values.each_with_index do |(_, value), i|
if Substitute === value
bound_attributes.each_with_index do |attr, i|
if Substitute === attr.value
@indexes << i
end
end
end

def bind(values)
bvs = @bind_values.map(&:dup)
@indexes.each_with_index { |offset,i| bvs[offset][1] = values[i] }
bvs
bas = @bound_attributes.dup
@indexes.each_with_index { |offset,i| bas[offset] = bas[offset].with_cast_value(values[i]) }
bas
end
end

attr_reader :bind_map, :query_builder

def self.create(connection, block = Proc.new)
relation = block.call Params.new
bind_map = BindMap.new relation.bind_values
bind_map = BindMap.new relation.bound_attributes
query_builder = connection.cacheable_query relation.arel
new query_builder, bind_map
end
Expand Down

0 comments on commit b06f64c

Please sign in to comment.