diff options
Diffstat (limited to 'activerecord')
10 files changed, 113 insertions, 34 deletions
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index bafd59fc02..c3ccb93ffd 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -338,7 +338,7 @@ module ActiveRecord klass = klass_name.constantize table_name = klass.quoted_table_name - primary_key = reflection.options[:primary_key] || klass.primary_key + primary_key = (reflection.options[:primary_key] || klass.primary_key).to_s column_type = klass.columns.detect{|c| c.name == primary_key}.type ids = _id_map.keys.map do |id| diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index ee2e2765bc..37dbff4061 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1880,7 +1880,7 @@ module ActiveRecord def initialize(base, associations, joins) @join_parts = [JoinBase.new(base, joins)] - @associations = associations + @associations = {} @reflections = [] @base_records_hash = {} @base_records_in_order = [] @@ -1949,30 +1949,58 @@ module ActiveRecord protected + def cache_joined_association(association) + associations = [] + parent = association.parent + while parent != join_base + associations.unshift(parent.reflection.name) + parent = parent.parent + end + ref = @associations + associations.each do |key| + ref = ref[key] + end + ref[association.reflection.name] ||= {} + end + def build(associations, parent = nil, join_type = Arel::InnerJoin) parent ||= join_parts.last case associations when Symbol, String reflection = parent.reflections[associations.to_s.intern] or raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" - @reflections << reflection - join_association = build_join_association(reflection, parent) - join_association.join_type = join_type - @join_parts << join_association + unless join_association = find_join_association(reflection, parent) + @reflections << reflection + join_association = build_join_association(reflection, parent) + join_association.join_type = join_type + @join_parts << join_association + cache_joined_association(join_association) + end + join_association when Array associations.each do |association| build(association, parent, join_type) end when Hash associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| - build(name, parent, join_type) - build(associations[name], nil, join_type) + join_association = build(name, parent, join_type) + build(associations[name], join_association, join_type) end else raise ConfigurationError, associations.inspect end end + def find_join_association(name_or_reflection, parent) + if String === name_or_reflection + name_or_reflection = name_or_reflection.to_sym + end + + join_associations.detect { |j| + j.reflection == name_or_reflection && j.parent == parent + } + end + def remove_uniq_by_reflection(reflection, records) if reflection && reflection.collection? records.each { |record| record.send(reflection.name).target.uniq! } @@ -2045,7 +2073,7 @@ module ActiveRecord end # A JoinPart represents a part of a JoinDependency. It is an abstract class, inherited - # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which + # by JoinBase and JoinAssociation. A JoinBase represents the Active Record which # everything else is being joined onto. A JoinAssociation represents an association which # is joining to the base. A JoinAssociation may result in more than one actual join # operations (for example a has_and_belongs_to_many JoinAssociation would result in @@ -2124,8 +2152,7 @@ module ActiveRecord def ==(other) other.class == self.class && - other.active_record == active_record && - other.table_joins == table_joins + other.active_record == active_record end def aliased_prefix @@ -2146,7 +2173,7 @@ module ActiveRecord attr_reader :reflection # The JoinDependency object which this JoinAssociation exists within. This is mainly - # relevant for generating aliases which do not conflict with other joins which are + # relevant for generating aliases which do not conflict with other joins which are # part of the query. attr_reader :join_dependency diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index a4d7d12298..fc4d84a4f2 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -226,7 +226,7 @@ module ActiveRecord IndexDefinition.new( table_name, row['name'], - row['unique'].to_i != 0, + row['unique'] != 0, exec("PRAGMA index_info('#{row['name']}')").map { |col| col['name'] }) @@ -235,7 +235,7 @@ module ActiveRecord def primary_key(table_name) #:nodoc: column = table_structure(table_name).find { |field| - field['pk'].to_i == 1 + field['pk'] == 1 } column && column['name'] end @@ -314,13 +314,12 @@ module ActiveRecord protected def select(sql, name = nil, binds = []) #:nodoc: - exec(sql, name, binds).map do |row| - record = {} - row.each do |key, value| - record[key.sub(/^"?\w+"?\./, '')] = value if key.is_a?(String) - end - record - end + result = exec(sql, name, binds) + columns = result.columns.map { |column| + column.sub(/^"?\w+"?\./, '') + } + + result.rows.map { |row| Hash[columns.zip(row)] } end def table_structure(table_name) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 58f7b74198..3b22be78cb 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -319,8 +319,13 @@ module ActiveRecord end def where_values_hash - Hash[@where_values.find_all {|w| w.respond_to?(:operator) && w.operator == :== }.map {|where| - [where.operand1.name, where.operand2.respond_to?(:value) ? where.operand2.value : where.operand2] + Hash[@where_values.find_all { |w| + w.respond_to?(:operator) && w.operator == :== && w.left.relation.name == table_name + }.map { |where| + [ + where.left.name, + where.right.respond_to?(:value) ? where.right.value : where.right + ] }] end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 648a02f1cc..a61a3bd41c 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -28,17 +28,20 @@ module ActiveRecord merged_wheres = @where_values + r.where_values - # Remove duplicates, last one wins. - seen = {} - merged_wheres = merged_wheres.reverse.reject { |w| - nuke = false - if w.respond_to?(:operator) && w.operator == :== - name = w.left.name - nuke = seen[name] - seen[name] = true - end - nuke - }.reverse + unless @where_values.empty? + # Remove duplicates, last one wins. + seen = Hash.new { |h,table| h[table] = {} } + merged_wheres = merged_wheres.reverse.reject { |w| + nuke = false + if w.respond_to?(:operator) && w.operator == :== + name = w.left.name + table = w.left.relation.name + nuke = seen[table][name] + seen[table][name] = true + end + nuke + }.reverse + end merged_relation.where_values = merged_wheres diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index cbaa4990f7..0fa4328826 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -81,6 +81,13 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_not_nil citibank_result.instance_variable_get("@firm_with_primary_key") end + def test_eager_loading_with_primary_key_as_symbol + Firm.create("name" => "Apple") + Client.create("name" => "Citibank", :firm_name => "Apple") + citibank_result = Client.find(:first, :conditions => {:name => "Citibank"}, :include => :firm_with_primary_key_symbols) + assert_not_nil citibank_result.instance_variable_get("@firm_with_primary_key_symbols") + end + def test_no_unexpected_aliasing first_firm = companies(:first_firm) another_firm = companies(:another_firm) diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb index 5b24d49a7d..81e99036af 100644 --- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb +++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb @@ -3,6 +3,7 @@ require 'models/post' require 'models/comment' require 'models/author' require 'models/categorization' +require 'models/category' require 'models/company' require 'models/topic' require 'models/reply' @@ -45,6 +46,31 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase assert_equal people(:michael), Person.eager_load(:primary_contact => :primary_contact).where('primary_contacts_people_2.first_name = ?', 'Susan').order('people.id').first end + def test_cascaded_eager_association_loading_with_join_for_count + categories = Category.joins(:categorizations).includes([{:posts=>:comments}, :authors]) + + assert_nothing_raised do + assert_equal 2, categories.count + assert_equal 2, categories.all.uniq.size # Must uniq since instantiating with inner joins will get dupes + end + end + + def test_cascaded_eager_association_loading_with_duplicated_includes + categories = Category.includes(:categorizations).includes(:categorizations => :author).where("categorizations.id is not null") + assert_nothing_raised do + assert_equal 2, categories.count + assert_equal 2, categories.all.size + end + end + + def test_cascaded_eager_association_loading_with_twice_includes_edge_cases + categories = Category.includes(:categorizations => :author).includes(:categorizations => :post).where("posts.id is not null") + assert_nothing_raised do + assert_equal 2, categories.count + assert_equal 2, categories.all.size + end + end + def test_eager_association_loading_with_join_for_count authors = Author.joins(:special_posts).includes([:posts, :categorizations]) diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index f3d3d62830..0ffd0e2ab3 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -226,6 +226,12 @@ class MethodScopingTest < ActiveRecord::TestCase assert Post.find(1).comments.include?(new_comment) end + def test_scoped_create_with_join_and_merge + (Comment.where(:body => "but Who's Buying?").joins(:post) & Post.where(:body => 'Peace Sells...')).with_scope do + assert_equal({:body => "but Who's Buying?"}, Comment.scoped.scope_for_create) + end + end + def test_immutable_scope options = { :conditions => "name = 'David'" } Developer.send(:with_scope, :find => options) do diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index b47ef7592d..268cea27be 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -500,6 +500,11 @@ class RelationTest < ActiveRecord::TestCase end end + def test_relation_merging_with_joins + comments = Comment.joins(:post).where(:body => 'Thank you for the welcome') & Post.where(:body => 'Such a lovely day') + assert_equal 1, comments.count + end + def test_count posts = Post.scoped diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index be6dd71e3b..ee5f77b613 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -107,6 +107,7 @@ class Client < Company belongs_to :firm_with_other_name, :class_name => "Firm", :foreign_key => "client_of" belongs_to :firm_with_condition, :class_name => "Firm", :foreign_key => "client_of", :conditions => ["1 = ?", 1] belongs_to :firm_with_primary_key, :class_name => "Firm", :primary_key => "name", :foreign_key => "firm_name" + belongs_to :firm_with_primary_key_symbols, :class_name => "Firm", :primary_key => :name, :foreign_key => :firm_name belongs_to :readonly_firm, :class_name => "Firm", :foreign_key => "firm_id", :readonly => true # Record destruction so we can test whether firm.clients.clear has |