From a3cf03ef99b2cfd811e0ca5cd31ef57976a9408b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 27 Feb 2012 10:57:40 -0800 Subject: use bind values for join columns This is a backport of 4bc2ae0da1dd812aee759f6d13ad428354cd0e13. It fixes bug #7950. Conflicts: activerecord/lib/active_record/relation/calculations.rb activerecord/lib/active_record/relation/finder_methods.rb --- .../lib/active_record/associations/association_scope.rb | 17 ++++++++++++++++- .../connection_adapters/abstract/database_statements.rb | 8 ++++---- activerecord/lib/active_record/relation.rb | 7 ++++++- activerecord/lib/active_record/relation/calculations.rb | 7 ++++--- .../lib/active_record/relation/finder_methods.rb | 4 ++-- .../lib/active_record/relation/spawn_methods.rb | 5 ++++- 6 files changed, 36 insertions(+), 12 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index f9cffa40c8..0b689cfc08 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -33,6 +33,18 @@ module ActiveRecord 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 @@ -67,7 +79,10 @@ module ActiveRecord conditions = self.conditions[i] if reflection == chain.last - scope = scope.where(table[key].eq(owner[foreign_key])) + 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)) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index db99c3fbef..174450eb00 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -20,14 +20,14 @@ module ActiveRecord # Returns a record hash with the column names as keys and column values # as values. - def select_one(arel, name = nil) - result = select_all(arel, name) + def select_one(arel, name = nil, binds = []) + result = select_all(arel, name, binds) result.first if result end # Returns a single value from a record - def select_value(arel, name = nil) - if result = select_one(arel, name) + def select_value(arel, name = nil, binds = []) + if result = select_one(arel, name, binds) result.values.first end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4b3b30d6ed..a727f40194 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -464,7 +464,12 @@ module ActiveRecord node.left.relation.name == table_name } - Hash[equalities.map { |where| [where.left.name, where.right] }] + 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 }] + }] end def scope_for_create diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 802059db21..8270292c43 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -178,7 +178,7 @@ module ActiveRecord # def pluck(column_name) column_name = column_name.to_s - klass.connection.select_all(select(column_name).arel).map! do |attributes| + klass.connection.select_all(select(column_name).arel, nil, bind_values).map! do |attributes| klass.type_cast_attribute(attributes.keys.first, klass.initialize_attributes(attributes)) end end @@ -240,7 +240,8 @@ module ActiveRecord query_builder = relation.arel end - type_cast_calculated_value(@klass.connection.select_value(query_builder), column_for(column_name), operation) + result = @klass.connection.select_value(query_builder, nil, relation.bind_values) + type_cast_calculated_value(result, column_for(column_name), operation) end def execute_grouped_calculation(operation, column_name, distinct) #:nodoc: @@ -286,7 +287,7 @@ module ActiveRecord relation = except(:group).group(group) relation.select_values = select_values - calculated_data = @klass.connection.select_all(relation) + calculated_data = @klass.connection.select_all(relation, nil, bind_values) if association key_ids = calculated_data.collect { |row| row[group_aliases.first] } diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index abc67d9c15..b31f74addc 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -199,7 +199,7 @@ module ActiveRecord relation = relation.where(table[primary_key].eq(id)) if id end - connection.select_value(relation, "#{name} Exists") ? true : false + connection.select_value(relation, "#{name} Exists", relation.bind_values) ? true : false rescue ThrowResult false end @@ -332,7 +332,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index c25570d758..3eead56e47 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -22,7 +22,7 @@ module ActiveRecord end end - (Relation::MULTI_VALUE_METHODS - [:joins, :where, :order]).each do |method| + (Relation::MULTI_VALUE_METHODS - [:joins, :where, :order, :binds]).each do |method| value = r.send(:"#{method}_values") merged_relation.send(:"#{method}_values=", merged_relation.send(:"#{method}_values") + value) if value.present? end @@ -31,6 +31,8 @@ module ActiveRecord 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] = {} } @@ -47,6 +49,7 @@ module ActiveRecord 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") -- cgit v1.2.3 From b6a2baee830f464344b7cb82caaa528be1db7389 Mon Sep 17 00:00:00 2001 From: George Brocklehurst Date: Thu, 25 Oct 2012 23:48:22 +0200 Subject: Test/changelog for has_many bug on unsaved records See issue #7950. The previous commit fixes this bug, and is a backport of 4bc2ae0da1dd812aee759f6d13ad428354cd0e13. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/test/cases/associations/has_many_associations_test.rb | 7 +++++++ 2 files changed, 13 insertions(+) (limited to 'activerecord') diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index bcc7765d2e..d3abe6062e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 3.2.10 (unreleased) +* 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* + * `AR::Base#attributes_before_type_cast` now returns unserialized values for serialized attributes. *Nikita Afanasenko* diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index ed4475770b..c311bf70d2 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1225,6 +1225,13 @@ class HasManyAssociationsTest < ActiveRecord::TestCase 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 -- cgit v1.2.3