Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slimmer #includes behaviour #1

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,38 @@
* Slimmer `#includes` behaviour.

The goal of the change is to make `#includes` slimmer - only join the table required to satisfy the referenced tables and preload all other associations independently.

The following two statements would demonstrate the difference BEFORE the change:

```sh
> Author.includes(:post, books: :essay).to_a

Author Load (0.2ms) SELECT "authors".* FROM "authors"
Post Load (0.3ms) SELECT "posts".* FROM "posts" WHERE "posts"."author_id" IN (?, ?, ?) [["author_id", 1], ["author_id", 2], ["author_id", 3]]
Book Load (0.3ms) SELECT "books".* FROM "books" WHERE "books"."author_id" IN (?, ?, ?) [["author_id", 1], ["author_id", 2], ["author_id", 3]]
Essay Load (0.1ms) SELECT "essays".* FROM "essays" WHERE "essays"."book_id" IN (?, ?, ?, ?) [["book_id", 1], ["book_id", 3], ["book_id", 2], ["book_id", 4]]

> Author.includes(:post, books: :essay).where(books: { language: 'EN' }).to_a

SQL (0.3ms) SELECT "authors"."id" AS t0_r0, "authors"."name" AS t0_r1, "authors"."author_address_id" AS t0_r2, "authors"."author_address_extra_id" AS t0_r3, "authors"."organization_id" AS t0_r4, "authors"."owned_essay_id" AS t0_r5, "posts"."id" AS t1_r0, "posts"."author_id" AS t1_r1, "posts"."title" AS t1_r2, "posts"."body" AS t1_r3, "posts"."type" AS t1_r4, "posts"."legacy_comments_count" AS t1_r5, "posts"."taggings_with_delete_all_count" AS t1_r6, "posts"."taggings_with_destroy_count" AS t1_r7, "posts"."tags_count" AS t1_r8, "posts"."indestructible_tags_count" AS t1_r9, "posts"."tags_with_destroy_count" AS t1_r10, "posts"."tags_with_nullify_count" AS t1_r11, "books"."id" AS t2_r0, "books"."author_id" AS t2_r1, "books"."format" AS t2_r2, "books"."format_record_id" AS t2_r3, "books"."format_record_type" AS t2_r4, "books"."name" AS t2_r5, "books"."status" AS t2_r6, "books"."last_read" AS t2_r7, "books"."nullable_status" AS t2_r8, "books"."language" AS t2_r9, "books"."author_visibility" AS t2_r10, "books"."illustrator_visibility" AS t2_r11, "books"."font_size" AS t2_r12, "books"."difficulty" AS t2_r13, "books"."cover" AS t2_r14, "books"."isbn" AS t2_r15, "books"."external_id" AS t2_r16, "books"."original_name" AS t2_r17, "books"."published_on" AS t2_r18, "books"."boolean_status" AS t2_r19, "books"."tags_count" AS t2_r20, "books"."created_at" AS t2_r21, "books"."updated_at" AS t2_r22, "books"."updated_on" AS t2_r23, "essays"."id" AS t3_r0, "essays"."type" AS t3_r1, "essays"."name" AS t3_r2, "essays"."writer_id" AS t3_r3, "essays"."writer_type" AS t3_r4, "essays"."category_id" AS t3_r5, "essays"."author_id" AS t3_r6, "essays"."book_id" AS t3_r7 FROM "authors" LEFT OUTER JOIN "posts" ON "posts"."author_id" = "authors"."id" LEFT OUTER JOIN "books" ON "books"."author_id" = "authors"."id" LEFT OUTER JOIN "essays" ON "essays"."book_id" = "books"."id" WHERE "books"."language" IS NULL

```

And this is AFTER the change:

```sh
> Author.includes(:post, books: :essay).where(books: { language: 'EN' }).to_a

SQL (0.2ms) SELECT "authors"."id" AS t0_r0, "authors"."name" AS t0_r1, "authors"."author_address_id" AS t0_r2, "authors"."author_address_extra_id" AS t0_r3, "authors"."organization_id" AS t0_r4, "authors"."owned_essay_id" AS t0_r5, "books"."id" AS t1_r0, "books"."author_id" AS t1_r1, "books"."format" AS t1_r2, "books"."format_record_id" AS t1_r3, "books"."format_record_type" AS t1_r4, "books"."name" AS t1_r5, "books"."status" AS t1_r6, "books"."last_read" AS t1_r7, "books"."nullable_status" AS t1_r8, "books"."language" AS t1_r9, "books"."author_visibility" AS t1_r10, "books"."illustrator_visibility" AS t1_r11, "books"."font_size" AS t1_r12, "books"."difficulty" AS t1_r13, "books"."cover" AS t1_r14, "books"."isbn" AS t1_r15, "books"."external_id" AS t1_r16, "books"."original_name" AS t1_r17, "books"."published_on" AS t1_r18, "books"."boolean_status" AS t1_r19, "books"."tags_count" AS t1_r20, "books"."created_at" AS t1_r21, "books"."updated_at" AS t1_r22, "books"."updated_on" AS t1_r23 FROM "authors" LEFT OUTER JOIN "books" ON "books"."author_id" = "authors"."id" WHERE "books"."language" IS NULL
Post Load (0.2ms) SELECT "posts".* FROM "posts" WHERE "posts"."author_id" IN (?, ?, ?) [["author_id", 1], ["author_id", 2], ["author_id", 3]]
Essay Load (0.1ms) SELECT "essays".* FROM "essays" WHERE "essays"."book_id" = ? [["book_id", 1]]

```

It avoids joining everything which could be performance-heavy.

*Artur Petrov, Svyatoslav Kryukov, Vladimir Dementyev*

* Remove deprecated `Tasks::DatabaseTasks.schema_file_type`.

*Rafael Mendonça França*
Expand Down
1 change: 1 addition & 0 deletions activerecord/lib/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ module ActiveRecord
autoload :Calculations
autoload :Delegation
autoload :FinderMethods
autoload :IncludesTracker
autoload :PredicateBuilder
autoload :QueryMethods
autoload :SpawnMethods
Expand Down
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Relation
VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS

include Enumerable
include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation
include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, IncludesTracker, Explain, Delegation

attr_reader :table, :klass, :loaded, :predicate_builder
attr_accessor :skip_preloading_value
Expand Down Expand Up @@ -838,7 +838,7 @@ def self.strict_loading_value

def preload_associations(records) # :nodoc:
preload = preload_values
preload += includes_values unless eager_loading?
preload += eager_loading? ? includes_values_non_referenced : includes_values
scope = strict_loading_value ? StrictLoadingScope : nil
preload.each do |associations|
ActiveRecord::Associations::Preloader.new(records: records, associations: associations, scope: scope).call
Expand Down
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/relation/calculations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def async_sum(identity_or_column = nil)
# end
def calculate(operation, column_name)
if has_include?(column_name)
relation = apply_join_dependency
relation = apply_join_dependency(full_eager_loading: true)
ardecvz marked this conversation as resolved.
Show resolved Hide resolved

if operation.to_s.downcase == "count"
unless distinct_value || distinct_select?(column_name || select_for_count)
Expand Down Expand Up @@ -251,7 +251,7 @@ def pluck(*column_names)
end

if has_include?(column_names.first)
relation = apply_join_dependency
relation = apply_join_dependency(full_eager_loading: true)
relation.pluck(*column_names)
else
klass.disallow_raw_sql!(column_names)
Expand Down Expand Up @@ -315,7 +315,7 @@ def ids
end

if has_include?(primary_key)
relation = apply_join_dependency.distinct
relation = apply_join_dependency(full_eager_loading: true).distinct
return relation.ids
end

Expand Down
5 changes: 3 additions & 2 deletions activerecord/lib/active_record/relation/finder_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,10 @@ def construct_relation_for_exists(conditions)
relation
end

def apply_join_dependency(eager_loading: group_values.empty?)
def apply_join_dependency(eager_loading: group_values.empty?, full_eager_loading: false)
selected_includes_values = full_eager_loading ? includes_values : includes_values_referenced
join_dependency = construct_join_dependency(
eager_load_values | includes_values, Arel::Nodes::OuterJoin
eager_load_values | selected_includes_values, Arel::Nodes::OuterJoin
)
relation = except(:includes, :eager_load, :preload).joins!(join_dependency)

Expand Down
90 changes: 90 additions & 0 deletions activerecord/lib/active_record/relation/includes_tracker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# frozen_string_literal: true

module ActiveRecord
# Determine includes which are used / not used in refererences and joins.
module IncludesTracker # :nodoc:
def includes_values_referenced
select_includes_values_with_references(:any?)
end

def includes_values_non_referenced
select_includes_values_with_references(:none?)
end

private
def select_includes_values_with_references(intersect_matcher)
all_references = (references_values + joins_values + left_outer_joins_values).map(&:to_s)

return includes_values if reference_numbered_table_alias?(all_references)

includes_tree = ActiveRecord::Associations::JoinDependency.make_tree(includes_values)
traverse_tree_with_model(includes_tree, self) do |reflection, parent_reflection|
next true unless reliable_reflection_match?(reflection)

possible_includes_tables = generate_includes_tables(reflection, parent_reflection)
(possible_includes_tables & all_references).public_send(intersect_matcher)
end
end

# Numbered table aliases bring ambiguity because of the digits at the end.
def reference_numbered_table_alias?(all_references)
all_references.any? { |reference| reference.last.between?("0", "9") }
end

# Possible includes tables contain:
# - Current table name;
# - All `through` table names;
# - Association name (`references` may set values directly);
# - Single alias for table;
# - Join table for HABTM;
# - Single alias for join table for HABTM.
def generate_includes_tables(reflection, parent_reflection)
all_possible_includes_tables =
reflection.collect_join_chain.map(&:table_name) <<
reflection.name.to_s <<
reflection.alias_candidate(reflection.table_name)

if inferable_reflection_table_name?(reflection)
all_possible_includes_tables <<
reflection.join_table <<
generate_possible_join_table_alias(reflection, parent_reflection)
end

all_possible_includes_tables
end

def generate_possible_join_table_alias(reflection, parent_reflection)
return unless parent_reflection

[reflection.join_table, parent_reflection.alias_candidate(:join)].sort.join("_")
end

def reliable_reflection_match?(reflection)
reflection && inferable_reflection_klass?(reflection)
end

def inferable_reflection_klass?(reflection)
!reflection.polymorphic?
end

def inferable_reflection_table_name?(reflection)
!reflection.through_reflection?
end

def traverse_tree_with_model(hash, model, parent_reflection = nil, &block)
return hash unless model

result = []
hash.each do |association, nested_hash|
current = model.reflect_on_association(association)

next_model = reliable_reflection_match?(current) ? current.klass : nil
current_result = traverse_tree_with_model(nested_hash, next_model, current, &block)
next (result << { association => current_result }) if current_result.any?

result << association if block.call(current, parent_reflection)
end
result
end
end
end
Loading