Skip to content

Commit

Permalink
Merge branch 'revert-binds-3-2' into 3-2-stable
Browse files Browse the repository at this point in the history
This has been resulting in some hard to track bugs and is introducing
a possible breackage in a stable version. The issue it currently "fixes"
should be handled in some other way.

Closes #8743. Related to #7983.
  • Loading branch information
carlosantoniodasilva committed Jan 15, 2013
2 parents 823604e + cfa4c64 commit 63970dc
Show file tree
Hide file tree
Showing 8 changed files with 12 additions and 49 deletions.
6 changes: 0 additions & 6 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,6 @@

*Victor Costan*

* Calling `include?` on `has_many` associations on unsaved records no longer
returns `true` when passed a record with a `nil` foreign key.
Fixes #7950.

*George Brocklehurst*

* `#pluck` can be used on a relation with `select` clause.
Fixes #7551.
Backport of #8176.
Expand Down
16 changes: 1 addition & 15 deletions activerecord/lib/active_record/associations/association_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,6 @@ def scope

private

def column_for(table_name, column_name)
columns = alias_tracker.connection.schema_cache.columns_hash[table_name]
columns[column_name]
end

def bind(scope, column, value)
substitute = alias_tracker.connection.substitute_at(
column, scope.bind_values.length)
scope.bind_values += [[column, value]]
substitute
end

def add_constraints(scope)
tables = construct_tables

Expand Down Expand Up @@ -79,9 +67,7 @@ def add_constraints(scope)
conditions = self.conditions[i]

if reflection == chain.last
column = column_for(table.table_name, key.to_s)
bind_val = bind(scope, column, owner[foreign_key])
scope = scope.where(table[key].eq(bind_val))
scope = scope.where(table[key].eq(owner[foreign_key]))

if reflection.type
scope = scope.where(table[reflection.type].eq(owner.class.base_class.name))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ module DatabaseStatements
# Converts an arel AST to SQL
def to_sql(arel, binds = [])
if arel.respond_to?(:ast)
binds = binds.dup
visitor.accept(arel.ast) do
quote(*binds.shift.reverse)
end
Expand All @@ -21,14 +20,14 @@ def select_all(arel, name = nil, binds = [])

# Returns a record hash with the column names as keys and column values
# as values.
def select_one(arel, name = nil, binds = [])
result = select_all(arel, name, binds)
def select_one(arel, name = nil)
result = select_all(arel, name)
result.first if result
end

# Returns a single value from a record
def select_value(arel, name = nil, binds = [])
if result = select_one(arel, name, binds)
def select_value(arel, name = nil)
if result = select_one(arel, name)
result.values.first
end
end
Expand Down
7 changes: 1 addition & 6 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,7 @@ def where_values_hash
node.left.relation.name == table_name
}

binds = Hash[bind_values.find_all(&:first).map { |column, v| [column.name, v] }]

Hash[equalities.map { |where|
name = where.left.name
[name, binds.fetch(name.to_s) { where.right }]
}]
Hash[equalities.map { |where| [where.left.name, where.right] }]
end

def scope_for_create
Expand Down
7 changes: 3 additions & 4 deletions activerecord/lib/active_record/relation/calculations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ def pluck(column_name)

relation = clone
relation.select_values = [column_name]
klass.connection.select_all(relation.arel, nil, bind_values).map! do |attributes|
klass.connection.select_all(relation.arel).map! do |attributes|
klass.type_cast_attribute(attributes.keys.first, klass.initialize_attributes(attributes))
end
end
Expand Down Expand Up @@ -247,8 +247,7 @@ def execute_simple_calculation(operation, column_name, distinct) #:nodoc:
query_builder = relation.arel
end

result = @klass.connection.select_value(query_builder, nil, relation.bind_values)
type_cast_calculated_value(result, column_for(column_name), operation)
type_cast_calculated_value(@klass.connection.select_value(query_builder), column_for(column_name), operation)
end

def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
Expand Down Expand Up @@ -294,7 +293,7 @@ def execute_grouped_calculation(operation, column_name, distinct) #:nodoc:
relation = except(:group).group(group)
relation.select_values = select_values

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

if association
key_ids = calculated_data.collect { |row| row[group_aliases.first] }
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def exists?(id = false)
relation = relation.where(table[primary_key].eq(id)) if id
end

connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false
connection.select_value(relation, "#{name} Exists") ? true : false
rescue ThrowResult
false
end
Expand Down Expand Up @@ -334,7 +334,7 @@ def find_one(id)

substitute = connection.substitute_at(column, @bind_values.length)
relation = where(table[primary_key].eq(substitute))
relation.bind_values += [[column, id]]
relation.bind_values = [[column, id]]
record = relation.first

unless record
Expand Down
5 changes: 1 addition & 4 deletions activerecord/lib/active_record/relation/spawn_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def merge(r)
end
end

(Relation::MULTI_VALUE_METHODS - [:joins, :where, :order, :binds]).each do |method|
(Relation::MULTI_VALUE_METHODS - [:joins, :where, :order]).each do |method|
value = r.send(:"#{method}_values")
merged_relation.send(:"#{method}_values=", merged_relation.send(:"#{method}_values") + value) if value.present?
end
Expand All @@ -31,8 +31,6 @@ def merge(r)

merged_wheres = @where_values + r.where_values

merged_binds = (@bind_values + r.bind_values).uniq(&:first)

unless @where_values.empty?
# Remove duplicates, last one wins.
seen = Hash.new { |h,table| h[table] = {} }
Expand All @@ -49,7 +47,6 @@ def merge(r)
end

merged_relation.where_values = merged_wheres
merged_relation.bind_values = merged_binds

(Relation::SINGLE_VALUE_METHODS - [:lock, :create_with, :reordering]).each do |method|
value = r.send(:"#{method}_value")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1225,13 +1225,6 @@ def test_included_in_collection
assert companies(:first_firm).clients.include?(Client.find(2))
end

def test_included_in_collection_for_new_records
client = Client.create(:name => 'Persisted')
assert_nil client.client_of
assert !Firm.new.clients_of_firm.include?(client),
'includes a client that does not belong to any firm'
end

def test_adding_array_and_collection
assert_nothing_raised { Firm.find(:first).clients + Firm.find(:all).last.clients }
end
Expand Down

0 comments on commit 63970dc

Please sign in to comment.