From b3d6f77f32072a96e68502b0488bfd6bea1128b4 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 28 Aug 2013 17:44:42 -0700 Subject: only call to_a when we have to --- .../lib/active_record/associations/preloader/association.rb | 6 +++++- .../active_record/associations/preloader/has_and_belongs_to_many.rb | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 0cc836f991..59aeb4eb6a 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -29,6 +29,10 @@ module ActiveRecord end def records_for(ids) + query_scope(ids).to_a + end + + def query_scope(ids) scope.where(association_key.in(ids)) end @@ -77,7 +81,7 @@ module ActiveRecord # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - records = sliced.flat_map { |slice| records_for(slice).to_a } + records = sliced.flat_map { |slice| records_for(slice) } end # Each record may have multiple owners, and vice-versa diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb index 9a3fada380..08a36db877 100644 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb @@ -12,7 +12,7 @@ module ActiveRecord # Unlike the other associations, we want to get a raw array of rows so that we can # access the aliased column on the join table def records_for(ids) - scope = super + scope = query_scope ids klass.connection.select_all(scope.arel, 'SQL', scope.bind_values) end -- cgit v1.2.3 From e7baa6624695d844e903193bcdda1b928bf61042 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 28 Aug 2013 17:47:53 -0700 Subject: extract owner id calculation to a method --- .../lib/active_record/associations/preloader/association.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 59aeb4eb6a..39a1d022a4 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -87,7 +87,7 @@ module ActiveRecord # Each record may have multiple owners, and vice-versa records_by_owner = Hash[owners.map { |owner| [owner, []] }] records.each do |record| - owner_key = record[association_key_name].to_s + owner_key = owner_id_for records, record owners_map[owner_key].each do |owner| records_by_owner[owner] << record @@ -96,6 +96,10 @@ module ActiveRecord records_by_owner end + def owner_id_for(results, record) + record[association_key_name].to_s + end + def reflection_scope @reflection_scope ||= reflection.scope ? klass.unscoped.instance_exec(nil, &reflection.scope) : klass.unscoped end -- cgit v1.2.3 From 98c085357e87ee49aec27d270f18502f2a062bef Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 28 Aug 2013 17:53:03 -0700 Subject: avoid extra empty array allocation --- .../associations/preloader/association.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 39a1d022a4..3059bae76f 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -75,24 +75,24 @@ module ActiveRecord owners_map = owners_by_key owner_keys = owners_map.keys.compact - if klass.nil? || owner_keys.empty? - records = [] - else + # Each record may have multiple owners, and vice-versa + records_by_owner = Hash[owners.map { |owner| [owner, []] }] + + if klass && owner_keys.any? # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) records = sliced.flat_map { |slice| records_for(slice) } - end - # Each record may have multiple owners, and vice-versa - records_by_owner = Hash[owners.map { |owner| [owner, []] }] - records.each do |record| - owner_key = owner_id_for records, record + records.each do |record| + owner_key = owner_id_for records, record - owners_map[owner_key].each do |owner| - records_by_owner[owner] << record + owners_map[owner_key].each do |owner| + records_by_owner[owner] << record + end end end + records_by_owner end -- cgit v1.2.3 From 242d70278689a0f7e6c6f415992c8de0aa5caee0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 28 Aug 2013 17:54:58 -0700 Subject: remove extra flat_map array --- .../active_record/associations/preloader/association.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 3059bae76f..dbcb1e75a9 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -82,15 +82,16 @@ module ActiveRecord # Some databases impose a limit on the number of ids in a list (in Oracle it's 1000) # Make several smaller queries if necessary or make one query if the adapter supports it sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) - records = sliced.flat_map { |slice| records_for(slice) } - - records.each do |record| - owner_key = owner_id_for records, record - - owners_map[owner_key].each do |owner| - records_by_owner[owner] << record + sliced.each { |slice| + records = records_for(slice) + records.each do |record| + owner_key = owner_id_for records, record + + owners_map[owner_key].each do |owner| + records_by_owner[owner] << record + end end - end + } end records_by_owner -- cgit v1.2.3 From c0c4d276885553b2bd6f911b28c128c67a64045c Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 28 Aug 2013 17:56:16 -0700 Subject: no need to to_a the scope --- activerecord/lib/active_record/associations/preloader/association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index dbcb1e75a9..3eb3407dea 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -29,7 +29,7 @@ module ActiveRecord end def records_for(ids) - query_scope(ids).to_a + query_scope(ids) end def query_scope(ids) -- cgit v1.2.3 From ef724e41a2e53ff786f2ff9a145815b29a985b4f Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Wed, 28 Aug 2013 18:09:00 -0700 Subject: correctly typecast keys, remove conditionals, reduce object allocations --- .../lib/active_record/associations/preloader/association.rb | 13 ++++++------- .../associations/preloader/has_and_belongs_to_many.rb | 5 +++++ 2 files changed, 11 insertions(+), 7 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 3eb3407dea..928da71eed 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -56,12 +56,9 @@ module ActiveRecord raise NotImplementedError end - # We're converting to a string here because postgres will return the aliased association - # key in a habtm as a string (for whatever reason) def owners_by_key @owners_by_key ||= owners.group_by do |owner| - key = owner[owner_key_name] - key && key.to_s + owner[owner_key_name] end end @@ -84,8 +81,9 @@ module ActiveRecord sliced = owner_keys.each_slice(klass.connection.in_clause_length || owner_keys.size) sliced.each { |slice| records = records_for(slice) + caster = type_caster(records, association_key_name) records.each do |record| - owner_key = owner_id_for records, record + owner_key = caster.call record[association_key_name] owners_map[owner_key].each do |owner| records_by_owner[owner] << record @@ -97,8 +95,9 @@ module ActiveRecord records_by_owner end - def owner_id_for(results, record) - record[association_key_name].to_s + IDENTITY = lambda { |value| value } + def type_caster(results, name) + IDENTITY end def reflection_scope diff --git a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb index 08a36db877..c042a44b21 100644 --- a/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/preloader/has_and_belongs_to_many.rb @@ -40,6 +40,11 @@ module ActiveRecord end end + def type_caster(results, name) + caster = results.column_types.fetch(name, results.identity_type) + lambda { |value| caster.type_cast value } + end + def build_scope super.joins(join).select(join_select) end -- cgit v1.2.3