From 590b045ee2c0906ff162e6658a184afb201865d7 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 25 May 2020 09:49:17 +0900 Subject: [PATCH] Refactor to lazy construct join table aliases Early constructed join table aliases are not always used for eager loading, and it is required refactoring to improve eager loading duplicated through associations. --- .../associations/alias_tracker.rb | 11 +++++++---- .../associations/join_dependency.rb | 19 +++---------------- .../join_dependency/join_association.rb | 10 +++------- activerecord/lib/active_record/relation.rb | 3 +-- 4 files changed, 14 insertions(+), 29 deletions(-) diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index ac90ba0137..a0ab89b4ac 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -6,9 +6,14 @@ module ActiveRecord module Associations # Keeps track of table aliases for ActiveRecord::Associations::JoinDependency class AliasTracker # :nodoc: - def self.create(connection, initial_table, joins) + def self.create(connection, initial_table, joins, aliases = nil) if joins.empty? - aliases = Hash.new(0) + aliases ||= Hash.new(0) + elsif aliases + default_proc = aliases.default_proc || proc { 0 } + aliases.default_proc = proc { |h, k| + h[k] = initial_count_for(connection, k, joins) + default_proc.call(h, k) + } else aliases = Hash.new { |h, k| h[k] = initial_count_for(connection, k, joins) @@ -32,8 +37,6 @@ def self.initial_count_for(connection, name, table_joins) ).size elsif join.is_a?(Arel::Nodes::Join) join.left.name == name ? 1 : 0 - elsif join.is_a?(Hash) - join[name] else raise ArgumentError, "joins list should be initialized by list of Arel::Nodes::Join" end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index fc4177936c..b6457d38e0 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -81,11 +81,9 @@ def reflections def join_constraints(joins_to_add, alias_tracker) @alias_tracker = alias_tracker - construct_tables!(join_root) joins = make_join_constraints(join_root, join_type) joins.concat joins_to_add.flat_map { |oj| - construct_tables!(oj.join_root) if join_root.match? oj.join_root walk(join_root, oj.join_root, oj.join_type) else @@ -157,12 +155,6 @@ def aliases } end - def construct_tables!(join_root) - join_root.each_children do |parent, child| - child.tables = table_aliases_for(parent, child) - end - end - def make_join_constraints(join_root, join_type) join_root.children.flat_map do |child| make_constraints(join_root, child, join_type) @@ -172,18 +164,13 @@ def make_join_constraints(join_root, join_type) def make_constraints(parent, child, join_type) foreign_table = parent.table foreign_klass = parent.base_klass - joins = child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) - joins.concat child.children.flat_map { |c| make_constraints(child, c, join_type) } - end - - def table_aliases_for(parent, node) - node.reflection.chain.map { |reflection| + child.join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) do |reflection| alias_tracker.aliased_table_for( reflection.table_name, - table_alias_for(reflection, parent, reflection != node.reflection), + table_alias_for(reflection, parent, reflection != child.reflection), reflection.klass.type_caster ) - } + end.concat child.children.flat_map { |c| make_constraints(child, c, join_type) } end def table_alias_for(reflection, parent, join) diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index 31d12fa2fa..1f1e1ba6b5 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -14,7 +14,6 @@ def initialize(reflection, children) super(reflection.klass, children) @reflection = reflection - @tables = nil end def match?(other) @@ -22,8 +21,10 @@ def match?(other) super && reflection == other.reflection end - def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) + def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker, &block) joins = [] + tables = reflection.chain.map(&block) + @table = tables.first # The chain starts with the target table, but we want to end with it here (makes # more sense in this context), so we reverse @@ -63,11 +64,6 @@ def join_constraints(foreign_table, foreign_klass, join_type, alias_tracker) joins end - def tables=(tables) - @tables = tables - @table = tables.first - end - def readonly? return @readonly if defined?(@readonly) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 4c137d0d59..784bea5966 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -749,8 +749,7 @@ def has_limit_or_offset? # :nodoc: end def alias_tracker(joins = [], aliases = nil) # :nodoc: - joins += [aliases] if aliases - ActiveRecord::Associations::AliasTracker.create(connection, table.name, joins) + ActiveRecord::Associations::AliasTracker.create(connection, table.name, joins, aliases) end class StrictLoadingScope