From 44372c2a2d0b350e5374bd32eccf1d9a6f5b9a7f Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Fri, 10 Mar 2023 14:01:07 +0300 Subject: [PATCH 1/7] Slimmer #includes behaviour The goal of the commit is to make #includes slimmer - only join the table required to satisfy the referenced tables and preload all other associations independently. Context The Active Record #includes method results in different behaviour depending on the other parts of the query: it may execute multiple queries (one per included association) or build a single JOIN query to load everything at once. Before the commit, it was "all or nothing" behind an eager load flag - one referenced table was enough to transform the whole query into one big JOIN. However, it's not always necessary to join all the tables except the ones with conditions. Normally, we could avoid joining everything (which could be performance-heavy), and execute a few queries instead. Idea The main idea is to remove referenced tables (which are #references, #where, #order or copied by #and, #or, #merge) from the final join query. So we would LEFT OUTER JOINed only includes tables with conditions on the query. At the same time, we have to bring non-referenced tables back to separate query preloads. Backward compatibility 1) Includes and table names don't always match so we use reflections. We try to match all possible includes table names against references including inner and left outer joins. If there's an unreliable match (like no reflection for association), we're inclined to JOIN the associations in order to not break user applications. All current tests are green without changes inside them. 2) The commit certainly breaks user code with the plain string where or select: ``` Author.includes(:company, :books).where(company: { title: "The Office" }) .where("books.title = ?", "Design Patterns") .select("books.title") ``` We do not want to introduce any flags or scopes to support this behaviour because in docs there're mentions to prohibit string causes without references: https://api.rubyonrails.org/v7.0.4.2/classes/ActiveRecord/QueryMethods.html#method-i-includes https://api.rubyonrails.org/v7.0.4.2/classes/ActiveRecord/QueryMethods.html#method-i-references People who don't know it have to start using it. Future improvements 1) Includes are not always plain - they may contain several levels inside a hash. In the commit, we handle several includes as separate trees starting from the top-level root. If there's any reference to possible includes table, we'll mark this whole tree as needed to JOINed. It's a safe bet which could be improved in future with a more fine-grained reference marking algorithm. 2) The commit's tree traversal is average performant at best. Several intermediate structures and tree walks could be removed - we prefer a simple algorithm at the moment to show the idea. --- activerecord/CHANGELOG.md | 31 ++ activerecord/lib/active_record.rb | 1 + activerecord/lib/active_record/relation.rb | 4 +- .../active_record/relation/calculations.rb | 6 +- .../active_record/relation/finder_methods.rb | 5 +- .../relation/includes_tracker.rb | 87 ++++++ .../eager_load_mixing_preload_test.rb | 270 ++++++++++++++++++ 7 files changed, 397 insertions(+), 7 deletions(-) create mode 100644 activerecord/lib/active_record/relation/includes_tracker.rb create mode 100644 activerecord/test/cases/associations/eager_load_mixing_preload_test.rb diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 023b5394754b8..7030ccaf9cd2e 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,34 @@ +* 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(:books, :post).to_a + + Author Load (0.1ms) SELECT "authors".* FROM "authors" + Book Load (0.3ms) SELECT "books".* FROM "books" WHERE "books"."author_id" IN (?, ?, ?) [["author_id", 1], ["author_id", 2], ["author_id", 3]] + Post Load (0.2ms) SELECT "posts".* FROM "posts" WHERE "posts"."author_id" IN (?, ?, ?) [["author_id", 1], ["author_id", 2], ["author_id", 3]] + + > Author.includes(:books, :post).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, "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, "posts"."id" AS t2_r0, "posts"."author_id" AS t2_r1, "posts"."team_id" AS t2_r2, "posts"."title" AS t2_r3, "posts"."body" AS t2_r4, "posts"."type" AS t2_r5, "posts"."legacy_comments_count" AS t2_r6, "posts"."taggings_with_delete_all_count" AS t2_r7, "posts"."taggings_with_destroy_count" AS t2_r8, "posts"."tags_count" AS t2_r9, "posts"."indestructible_tags_count" AS t2_r10, "posts"."tags_with_destroy_count" AS t2_r11, "posts"."tags_with_nullify_count" AS t2_r12 FROM "authors" LEFT OUTER JOIN "books" ON "books"."author_id" = "authors"."id" LEFT OUTER JOIN "posts" ON "posts"."author_id" = "authors"."id" WHERE "books"."language" IS NULL + + ``` + + And this is AFTER the change: + + ```sh + > Author.includes(:books, :post).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", 2], ["author_id", 3]] + + ``` + + *Artur Petrov* + * Remove deprecated `Tasks::DatabaseTasks.schema_file_type`. *Rafael Mendonça França* diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 2137f7912923e..c65f7508043f5 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -111,6 +111,7 @@ module ActiveRecord autoload :Calculations autoload :Delegation autoload :FinderMethods + autoload :IncludesTracker autoload :PredicateBuilder autoload :QueryMethods autoload :SpawnMethods diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 8dc77626127a6..e2c188d3bc4df 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -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 @@ -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 diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 4ec950d9aa8e5..a3411de853f28 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -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) if operation.to_s.downcase == "count" unless distinct_value || distinct_select?(column_name || select_for_count) @@ -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) @@ -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 diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 2f8c2a590d8e7..870342e6f755a 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -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) diff --git a/activerecord/lib/active_record/relation/includes_tracker.rb b/activerecord/lib/active_record/relation/includes_tracker.rb new file mode 100644 index 0000000000000..74dc9d7e227b1 --- /dev/null +++ b/activerecord/lib/active_record/relation/includes_tracker.rb @@ -0,0 +1,87 @@ +# 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?, :present?) + end + + def includes_values_non_referenced + select_includes_values_with_references(:all?, :blank?) + end + + private + def select_includes_values_with_references(matcher, intersect_matcher) + all_references = (references_values + joins_values + left_outer_joins_values).map(&:to_s) + + normalized_includes_values.select do |includes_value| + includes_tree = ActiveRecord::Associations::JoinDependency.make_tree(includes_value) + + includes_values_reflections(includes_tree).public_send(matcher) do |reflection| + next true unless reliable_reflection_match?(reflection) + + (possible_includes_tables(reflection) & all_references).public_send(intersect_matcher) + end + end + end + + def includes_values_reflections(includes_tree) + includes_reflections = [] + + traverse_tree_with_model(includes_tree, self) do |association, model| + reflection = model.reflect_on_association(association) + + includes_reflections << reflection + + reliable_reflection_match?(reflection) ? reflection.klass : model + end + + includes_reflections + end + + def possible_includes_tables(reflection) + all_possible_includes_tables = + reflection.collect_join_chain.map(&:table_name) << + reflection.alias_candidate(reflection.table_name) << + reflection.name.to_s + + if inferable_reflection_table_name?(reflection) + all_possible_includes_tables << + reflection.join_table << + join_table_with_postfix_alias(reflection) + end + + all_possible_includes_tables + 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 join_table_with_postfix_alias(reflection) + [reflection.join_table, reflection.alias_candidate(:join)].sort.join("_") + end + + def normalized_includes_values + includes_values.map do |element| + element.is_a?(Hash) ? element.map { |key, value| { key => value } } : element + end.flatten + end + + def traverse_tree_with_model(object, model, &block) + object.each do |key, value| + next_model = yield(key, model) + traverse_tree_with_model(value, next_model, &block) if next_model + end + end + end +end diff --git a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb new file mode 100644 index 0000000000000..36081720dd4cc --- /dev/null +++ b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb @@ -0,0 +1,270 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/author" +require "models/book" +require "models/citation" +require "models/reader" +require "models/post" +require "models/reference" +require "models/comment" +require "models/rating" +require "models/category" +require "models/categorization" +require "models/tag" +require "models/tagging" +require "models/person" +require "models/club" +require "models/developer" +require "models/project" +require "models/computer" +require "models/company" +require "models/contract" +require "models/member" +require "models/membership" +require "models/sponsor" + +class AssociationsEagerLoadMixingPreloadTest < ActiveRecord::TestCase + fixtures :authors, :books, :citations, :readers, :posts, :references, :comments, :ratings, + :categories, :categorizations, :tags, :taggings, :people, :clubs, :members, :memberships, + :sponsors, :developers, :projects, :developers_projects, :computers, :companies, :accounts + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_has_one_association + authors = Author.includes(:books, :post).where(post: { type: "Post" }) + assert_left_joins(1) { authors.to_sql } + assert_queries(2) { authors.to_a } + assert_no_queries { authors.map(&:post).map(&:title) } + assert_no_queries { authors.flat_map(&:books).map(&:title) } + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_has_many_association + authors = Author.includes(:books, :post).where(books: { language: nil }) + assert_left_joins(1) { authors.to_sql } + assert_queries(2) { authors.to_a } + assert_no_queries { authors.map(&:post).map(&:title) } + assert_no_queries { authors.flat_map(&:books).map(&:title) } + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_habtm_association + developers = Developer.includes(:audit_logs, :shared_computers) + .where(shared_computers: { timezone: nil }) + assert_left_joins(2) { developers.to_sql } + assert_queries(2) { developers.to_a } + assert_no_queries { developers.flat_map(&:audit_logs).map(&:message) } + assert_no_queries { developers.flat_map(&:shared_computers).map(&:system) } + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_has_many_through_association + authors = Author.includes(:ratings, :post).where(ratings: { value: 1 }) + assert_left_joins(3) { authors.to_sql } + assert_queries(2) { authors.to_a } + assert_no_queries { authors.map(&:post).map(&:title) } + assert_no_queries { authors.flat_map(&:ratings).map(&:value) } + end + + def test_mixing_eager_load_and_preload_for_referenced_only_indirect_through_association + persons = Person.includes(:posts, :references).references(:readers) + .where("readers.id = 1 or 1=1") + assert_left_joins(2) { persons.to_sql } + assert_queries(2) { persons.to_a } + assert_queries(3) { persons.flat_map(&:readers).map(&:skimmer) } + assert_no_queries { persons.flat_map(&:posts).map(&:title) } + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_second_level_association + authors = Author.includes(books: :citations).where(citations: { citation_id: nil }) + assert_left_joins(2) { authors.to_sql } + assert_queries(1) { authors.to_a } + assert_no_queries { authors.flat_map(&:books).map(&:title) } + assert_no_queries { authors.flat_map(&:books).flat_map(&:citations).map(&:citation) } + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_several_levels_association + authors = Author.includes( + :books, { posts: :special_comments }, { categorizations: :category } + ).order("comments.body").where("posts.id = 4") + assert_left_joins(2) { authors.to_sql } + assert_queries(4) { authors.to_a } + assert_no_queries do + authors.first.books.first + authors.first.posts.first.special_comments.first + authors.first.categorizations.first.category + end + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_standard_hash_association + authors = Author.includes( + :books, posts: :special_comments, categorizations: :category + ).order("comments.body").where("posts.id = 4") + assert_left_joins(2) { authors.to_sql } + assert_queries(4) { authors.to_a } + assert_no_queries do + authors.first.books.first + authors.first.posts.first.special_comments.first + authors.first.categorizations.first.category + end + end + + def test_mixing_eager_load_and_preload_for_deep_nested_includes + authors = Author.includes( + :books, posts: { special_comments: { post: [ :special_comments, :very_special_comment ] } } + ).order("comments.body", "very_special_comments_posts.body").where("posts.id = 4") + assert_left_joins(5) { authors.to_sql } + assert_queries(2) { authors.to_a } + assert_no_queries do + authors.first.books.first + authors.first.posts.first.special_comments.first.post.special_comments + authors.first.posts.first.special_comments.first.post.very_special_comment + end + end + + def test_mixing_eager_load_and_preload_for_includes_with_referenced_single_argument_association + authors = Author.includes(:books).where(books: { language: nil }) + assert_left_joins(1) { authors.to_sql } + assert_queries(1) { authors.to_a } + assert_no_queries { authors.flat_map(&:books).map(&:title) } + end + + def test_mixing_eager_load_and_preload_for_joined_includes + posts = Post.includes(:comments, :author).joins(:comments).left_joins(:author) + assert_left_joins(2) { posts.to_sql } + assert_queries(1) { posts.to_a } + assert_no_queries { posts.flat_map(&:comments).map(&:type) } + assert_no_queries { posts.map(&:author).map(&:name) } + end + + def test_mixing_eager_load_and_preload_for_eager_loaded_includes + authors = Author.eager_load(:posts).includes(posts: :special_comments) + assert_left_joins(1) { authors.to_sql } + assert_queries(2) { authors.to_a } + assert_no_queries { authors.flat_map(&:posts).map(&:title) } + assert_no_queries { authors.flat_map(&:posts).flat_map(&:special_comments).map(&:type) } + end + + def test_mixing_eager_load_and_preload_for_preloaded_includes + authors = Author.preload(:posts).includes(posts: :special_comments) + assert_left_joins(0) { authors.to_sql } + assert_queries(3) { authors.to_a } + assert_no_queries { authors.flat_map(&:posts).map(&:title) } + assert_no_queries { authors.flat_map(&:posts).flat_map(&:special_comments).map(&:type) } + end + + def test_mixing_eager_load_and_preload_without_eager_loading + persons = Person.males.includes(:agents) + assert_left_joins(0) { persons.to_sql } + assert_queries(2) { persons.to_a } + assert_no_queries { persons.flat_map(&:agents).map(&:first_name) } + end + + def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_calculation + companies = Company.includes(:contracts) + assert_sql(/LEFT OUTER JOIN/) { companies.sum(:developer_id) } + assert_queries(1) { companies.sum(:developer_id) } + end + + def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_pluck + companies = Company.includes(:contracts) + assert_sql(/LEFT OUTER JOIN/) { companies.pluck(:developer_id) } + assert_queries(1) { companies.pluck(:developer_id) } + end + + def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_ids + companies = Company.includes(:contracts, account: :firm) + assert_sql(/LEFT OUTER JOIN/) { companies.ids } + assert_queries(1) { companies.ids } + end + + def test_mixing_eager_load_and_preload_with_unreliable_non_existent_reflection_match + developers = Developer.all + .includes(:developers_projects) + .where("developers_projects.joined_on": nil) + assert_left_joins(1) { developers.to_sql } + assert_queries(1) { developers.to_a } + end + + def test_mixing_eager_load_and_preload_with_uninferable_reflection_klass + assert_raise ActiveRecord::EagerLoadPolymorphicError do + tags(:general).taggings + .includes(:taggable).references(:bogus_table) + .to_a + end + end + + def test_mixing_eager_load_and_preload_with_uninferable_reflection_table_name + members = Member.all + .includes(:sponsor_club) + .where("members.name": "Groucho Marx") + .order("clubs.name") + assert_left_joins(2) { members.to_sql } + assert_queries(1) { members.to_a } + assert_no_queries { members.map(&:sponsor_club).map(&:name) } + end + + def test_mixing_eager_load_and_preload_table_alias + firms = Firm.all + .includes(:clients_using_primary_key) + .order("clients_using_primary_keys_companies.name") + assert_left_joins(1) { firms.to_sql } + assert_queries(1) { firms.to_a } + assert_no_queries { firms.flat_map(&:clients_using_primary_key).map(&:name) } + end + + def test_mixing_eager_load_and_preload_join_table + developers = Developer.all + .includes(:projects) + .where("developers_projects.access_level": 1) + assert_left_joins(2) { developers.to_sql } + assert_queries(1) { developers.to_a } + assert_no_queries { developers.flat_map(&:projects).map(&:name) } + end + + def test_mixing_eager_load_and_preload_join_table_with_postfix_alias + developers = Developer.all + .includes(projects: :developers) + .where("developers_projects_projects_join.access_level.joined_on": nil) + assert_left_joins(4) { developers.to_sql } + assert_queries(1) { developers.to_a } + assert_no_queries { developers.flat_map(&:projects).map(&:name) } + assert_no_queries { developers.flat_map(&:projects).flat_map(&:developers).map(&:first_name) } + end + + def test_mixing_eager_load_and_preload_for_cache_key_with_non_referenced_includes + accounts = Account.includes(:firm) + assert_sql(/^(?!.*LEFT OUTER JOIN).*/) { accounts.cache_key } + assert_no_queries { accounts.cache_key } + end + + def test_mixing_eager_load_and_preload_within_update_all + accounts = Account.includes(:firm) + assert_sql(/^(?!.*LEFT OUTER JOIN).*/) { accounts.update_all("firm_name = 1") } + assert_queries(1) { accounts.update_all("firm_name = 1") } + end + + def test_mixing_eager_load_and_preload_within_delete_all + accounts = Account.includes(:firm) + assert_sql(/^(?!.*LEFT OUTER JOIN).*/) { accounts.delete_all } + assert_queries(1) { accounts.delete_all } + end + + def test_mixing_eager_load_and_preload_for_non_existent_association + projects = Project.includes(:non_existent).references(:non_existent) + assert_raises(ActiveRecord::ConfigurationError) do + projects.to_a + end + end + + def test_mixing_eager_load_and_preload_for_non_existent_column + authors = Author.includes(books: :citations).where(citations: { citation: nil }) + assert_raises(ActiveRecord::StatementInvalid) do + authors.to_a + end + end + + private + def assert_left_joins(num = 1, &block) + sql = _assert_nothing_raised_or_warn("assert_left_joins", &block) + count = sql.scan("LEFT OUTER JOIN").count + msg = "#{count} instead of #{num} LEFT JOINs were included\nSQL:\n#{sql}" + assert_equal num, count, msg + end +end From c5c1f43b5127932d3bb8eb2a2fdc1444458141e9 Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Tue, 14 Mar 2023 00:09:54 +0300 Subject: [PATCH 2/7] Rewrite the tree traversal to support an example from README --- .../relation/includes_tracker.rb | 51 +++++++------------ .../eager_load_mixing_preload_test.rb | 21 +++++--- .../nested_through_associations_test.rb | 2 +- 3 files changed, 34 insertions(+), 40 deletions(-) diff --git a/activerecord/lib/active_record/relation/includes_tracker.rb b/activerecord/lib/active_record/relation/includes_tracker.rb index 74dc9d7e227b1..c8a00ca0bac79 100644 --- a/activerecord/lib/active_record/relation/includes_tracker.rb +++ b/activerecord/lib/active_record/relation/includes_tracker.rb @@ -4,42 +4,26 @@ 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?, :present?) + select_includes_values_with_references(:present?) end def includes_values_non_referenced - select_includes_values_with_references(:all?, :blank?) + select_includes_values_with_references(:blank?) end private - def select_includes_values_with_references(matcher, intersect_matcher) + def select_includes_values_with_references(intersect_matcher) all_references = (references_values + joins_values + left_outer_joins_values).map(&:to_s) - normalized_includes_values.select do |includes_value| - includes_tree = ActiveRecord::Associations::JoinDependency.make_tree(includes_value) + includes_tree = ActiveRecord::Associations::JoinDependency.make_tree(includes_values) - includes_values_reflections(includes_tree).public_send(matcher) do |reflection| - next true unless reliable_reflection_match?(reflection) + traverse_tree_with_model(includes_tree, self) do |reflection| + next true unless reliable_reflection_match?(reflection) - (possible_includes_tables(reflection) & all_references).public_send(intersect_matcher) - end + (possible_includes_tables(reflection) & all_references).public_send(intersect_matcher) end end - def includes_values_reflections(includes_tree) - includes_reflections = [] - - traverse_tree_with_model(includes_tree, self) do |association, model| - reflection = model.reflect_on_association(association) - - includes_reflections << reflection - - reliable_reflection_match?(reflection) ? reflection.klass : model - end - - includes_reflections - end - def possible_includes_tables(reflection) all_possible_includes_tables = reflection.collect_join_chain.map(&:table_name) << @@ -71,17 +55,20 @@ def join_table_with_postfix_alias(reflection) [reflection.join_table, reflection.alias_candidate(:join)].sort.join("_") end - def normalized_includes_values - includes_values.map do |element| - element.is_a?(Hash) ? element.map { |key, value| { key => value } } : element - end.flatten - end + def traverse_tree_with_model(hash, model, &block) + return hash if model.nil? + + 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, &block) + next (result << { association => current_result }) if current_result.any? - def traverse_tree_with_model(object, model, &block) - object.each do |key, value| - next_model = yield(key, model) - traverse_tree_with_model(value, next_model, &block) if next_model + result << association if block.call(current) end + result end end end diff --git a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb index 36081720dd4cc..f70d1769b3897 100644 --- a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb +++ b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb @@ -62,6 +62,13 @@ def test_mixing_eager_load_and_preload_for_includes_with_referenced_has_many_thr assert_no_queries { authors.flat_map(&:ratings).map(&:value) } end + def test_mixing_eager_load_and_preload_for_includes_with_advanced_has_many_through_association + authors = Author.includes(posts: :categorizations).where(posts: { tags_count: 1 }) + assert_left_joins(1) { authors.to_sql } + assert_queries(2) { authors.to_a } + assert_no_queries { authors.flat_map(&:posts).flat_map(&:categorizations).map(&:special) } + end + def test_mixing_eager_load_and_preload_for_referenced_only_indirect_through_association persons = Person.includes(:posts, :references).references(:readers) .where("readers.id = 1 or 1=1") @@ -158,14 +165,14 @@ def test_mixing_eager_load_and_preload_without_eager_loading def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_calculation companies = Company.includes(:contracts) - assert_sql(/LEFT OUTER JOIN/) { companies.sum(:developer_id) } - assert_queries(1) { companies.sum(:developer_id) } + assert_sql(/LEFT OUTER JOIN/) { companies.sum(:metadata) } + assert_queries(1) { companies.sum(:metadata) } end def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_pluck companies = Company.includes(:contracts) - assert_sql(/LEFT OUTER JOIN/) { companies.pluck(:developer_id) } - assert_queries(1) { companies.pluck(:developer_id) } + assert_sql(/LEFT OUTER JOIN/) { companies.pluck(:metadata) } + assert_queries(1) { companies.pluck(:metadata) } end def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_ids @@ -212,7 +219,7 @@ def test_mixing_eager_load_and_preload_table_alias def test_mixing_eager_load_and_preload_join_table developers = Developer.all .includes(:projects) - .where("developers_projects.access_level": 1) + .where("developers_projects.joined_on": nil) assert_left_joins(2) { developers.to_sql } assert_queries(1) { developers.to_a } assert_no_queries { developers.flat_map(&:projects).map(&:name) } @@ -221,8 +228,8 @@ def test_mixing_eager_load_and_preload_join_table def test_mixing_eager_load_and_preload_join_table_with_postfix_alias developers = Developer.all .includes(projects: :developers) - .where("developers_projects_projects_join.access_level.joined_on": nil) - assert_left_joins(4) { developers.to_sql } + .where("developers_projects_projects_join.joined_on": nil) + assert_left_joins(2) { developers.to_sql } assert_queries(1) { developers.to_a } assert_no_queries { developers.flat_map(&:projects).map(&:name) } assert_no_queries { developers.flat_map(&:projects).flat_map(&:developers).map(&:first_name) } diff --git a/activerecord/test/cases/associations/nested_through_associations_test.rb b/activerecord/test/cases/associations/nested_through_associations_test.rb index aca3f3d83f3a0..d3aff2a4b91b4 100644 --- a/activerecord/test/cases/associations/nested_through_associations_test.rb +++ b/activerecord/test/cases/associations/nested_through_associations_test.rb @@ -539,7 +539,7 @@ def test_nested_has_many_through_with_conditions_on_through_associations def test_nested_has_many_through_with_conditions_on_through_associations_preload assert_empty Author.where("tags.id" => 100).joins(:misc_post_first_blue_tags) - author = assert_queries(2) { Author.includes(:misc_post_first_blue_tags).third } + author = assert_queries(3) { Author.includes(:misc_post_first_blue_tags).third } blue = tags(:blue) assert_no_queries do From 933104524c757e57e3c8d8659f8ba878215731d2 Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Tue, 14 Mar 2023 02:50:20 +0300 Subject: [PATCH 3/7] Fix specs --- .../relation/includes_tracker.rb | 23 ++++++++++++------- .../eager_load_mixing_preload_test.rb | 6 ++--- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/activerecord/lib/active_record/relation/includes_tracker.rb b/activerecord/lib/active_record/relation/includes_tracker.rb index c8a00ca0bac79..8fb4e6cd34fe2 100644 --- a/activerecord/lib/active_record/relation/includes_tracker.rb +++ b/activerecord/lib/active_record/relation/includes_tracker.rb @@ -3,6 +3,8 @@ module ActiveRecord # Determine includes which are used / not used in refererences and joins. module IncludesTracker # :nodoc: + TABLE_WITH_POSTFIX_ALIAS = "_join" + def includes_values_referenced select_includes_values_with_references(:present?) end @@ -15,8 +17,9 @@ def includes_values_non_referenced def select_includes_values_with_references(intersect_matcher) all_references = (references_values + joins_values + left_outer_joins_values).map(&:to_s) - includes_tree = ActiveRecord::Associations::JoinDependency.make_tree(includes_values) + return includes_values if references_table_with_postfix_alias?(all_references) + includes_tree = ActiveRecord::Associations::JoinDependency.make_tree(includes_values) traverse_tree_with_model(includes_tree, self) do |reflection| next true unless reliable_reflection_match?(reflection) @@ -24,6 +27,12 @@ def select_includes_values_with_references(intersect_matcher) end end + # Possible includes tables contain: + # - Current table name; + # - All `through` table names; + # - Special plural table name; + # - Association name (`references` may save directly); + # - Join table for HABTM. def possible_includes_tables(reflection) all_possible_includes_tables = reflection.collect_join_chain.map(&:table_name) << @@ -31,14 +40,16 @@ def possible_includes_tables(reflection) reflection.name.to_s if inferable_reflection_table_name?(reflection) - all_possible_includes_tables << - reflection.join_table << - join_table_with_postfix_alias(reflection) + all_possible_includes_tables << reflection.join_table end all_possible_includes_tables end + def references_table_with_postfix_alias?(all_references) + all_references.any? { _1.end_with?(TABLE_WITH_POSTFIX_ALIAS) } + end + def reliable_reflection_match?(reflection) reflection && inferable_reflection_klass?(reflection) end @@ -51,10 +62,6 @@ def inferable_reflection_table_name?(reflection) !reflection.through_reflection? end - def join_table_with_postfix_alias(reflection) - [reflection.join_table, reflection.alias_candidate(:join)].sort.join("_") - end - def traverse_tree_with_model(hash, model, &block) return hash if model.nil? diff --git a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb index f70d1769b3897..8664cd54fa580 100644 --- a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb +++ b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb @@ -229,10 +229,10 @@ def test_mixing_eager_load_and_preload_join_table_with_postfix_alias developers = Developer.all .includes(projects: :developers) .where("developers_projects_projects_join.joined_on": nil) - assert_left_joins(2) { developers.to_sql } - assert_queries(1) { developers.to_a } + assert_left_joins(4) { developers.to_sql } + assert_queries(5) { developers.to_a } assert_no_queries { developers.flat_map(&:projects).map(&:name) } - assert_no_queries { developers.flat_map(&:projects).flat_map(&:developers).map(&:first_name) } + assert_no_queries { developers.flat_map(&:projects).flat_map(&:developers).map(&:name) } end def test_mixing_eager_load_and_preload_for_cache_key_with_non_referenced_includes From 95ad72ba0ee2bcb5f46e1a79977ad344b8ae56d3 Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Tue, 14 Mar 2023 03:06:44 +0300 Subject: [PATCH 4/7] Fix PG type mismatch in function --- .../test/cases/associations/eager_load_mixing_preload_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb index 8664cd54fa580..9a15d75f112ee 100644 --- a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb +++ b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb @@ -165,8 +165,8 @@ def test_mixing_eager_load_and_preload_without_eager_loading def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_calculation companies = Company.includes(:contracts) - assert_sql(/LEFT OUTER JOIN/) { companies.sum(:metadata) } - assert_queries(1) { companies.sum(:metadata) } + assert_sql(/LEFT OUTER JOIN/) { companies.sum(:count) } + assert_queries(1) { companies.sum(:count) } end def test_mixing_eager_load_and_preload_with_forced_full_eager_loading_pluck From e53e3e92b3fa10c2df119f16a558fbb57259af91 Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Tue, 14 Mar 2023 20:45:43 +0300 Subject: [PATCH 5/7] Explain and cover one more case --- .../relation/includes_tracker.rb | 45 +++++++++++-------- .../eager_load_mixing_preload_test.rb | 28 ++++++++---- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/activerecord/lib/active_record/relation/includes_tracker.rb b/activerecord/lib/active_record/relation/includes_tracker.rb index 8fb4e6cd34fe2..4423ad76b286b 100644 --- a/activerecord/lib/active_record/relation/includes_tracker.rb +++ b/activerecord/lib/active_record/relation/includes_tracker.rb @@ -3,8 +3,6 @@ module ActiveRecord # Determine includes which are used / not used in refererences and joins. module IncludesTracker # :nodoc: - TABLE_WITH_POSTFIX_ALIAS = "_join" - def includes_values_referenced select_includes_values_with_references(:present?) end @@ -17,37 +15,48 @@ def includes_values_non_referenced 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 references_table_with_postfix_alias?(all_references) + 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| + traverse_tree_with_model(includes_tree, self) do |reflection, parent_reflection| next true unless reliable_reflection_match?(reflection) - (possible_includes_tables(reflection) & all_references).public_send(intersect_matcher) + 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; - # - Special plural table name; - # - Association name (`references` may save directly); - # - Join table for HABTM. - def possible_includes_tables(reflection) + # - 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.alias_candidate(reflection.table_name) << - reflection.name.to_s + reflection.name.to_s << + reflection.alias_candidate(reflection.table_name) if inferable_reflection_table_name?(reflection) - all_possible_includes_tables << reflection.join_table + all_possible_includes_tables << + reflection.join_table << + generate_possible_join_table_alias(reflection, parent_reflection) end all_possible_includes_tables end - def references_table_with_postfix_alias?(all_references) - all_references.any? { _1.end_with?(TABLE_WITH_POSTFIX_ALIAS) } + 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) @@ -62,18 +71,18 @@ def inferable_reflection_table_name?(reflection) !reflection.through_reflection? end - def traverse_tree_with_model(hash, model, &block) - return hash if model.nil? + 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, &block) + 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) + result << association if block.call(current, parent_reflection) end result end diff --git a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb index 9a15d75f112ee..5de4205324d60 100644 --- a/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb +++ b/activerecord/test/cases/associations/eager_load_mixing_preload_test.rb @@ -207,6 +207,15 @@ def test_mixing_eager_load_and_preload_with_uninferable_reflection_table_name assert_no_queries { members.map(&:sponsor_club).map(&:name) } end + def test_mixing_eager_load_and_preload_join_table + developers = Developer.all + .includes(:projects) + .where("developers_projects.joined_on": nil) + assert_left_joins(2) { developers.to_sql } + assert_queries(1) { developers.to_a } + assert_no_queries { developers.flat_map(&:projects).map(&:name) } + end + def test_mixing_eager_load_and_preload_table_alias firms = Firm.all .includes(:clients_using_primary_key) @@ -216,21 +225,22 @@ def test_mixing_eager_load_and_preload_table_alias assert_no_queries { firms.flat_map(&:clients_using_primary_key).map(&:name) } end - def test_mixing_eager_load_and_preload_join_table + def test_mixing_eager_load_and_preload_join_table_alias developers = Developer.all - .includes(:projects) - .where("developers_projects.joined_on": nil) - assert_left_joins(2) { developers.to_sql } + .includes(projects: :developers) + .where("developers_projects_projects_join.joined_on": nil) + assert_left_joins(4) { developers.to_sql } assert_queries(1) { developers.to_a } assert_no_queries { developers.flat_map(&:projects).map(&:name) } + assert_no_queries { developers.flat_map(&:projects).flat_map(&:developers).map(&:name) } end - def test_mixing_eager_load_and_preload_join_table_with_postfix_alias + def test_mixing_eager_load_and_preload_numbered_table_aliases developers = Developer.all - .includes(projects: :developers) - .where("developers_projects_projects_join.joined_on": nil) - assert_left_joins(4) { developers.to_sql } - assert_queries(5) { developers.to_a } + .includes(projects: { developers: :projects }) + .where("developers_projects_2.salary": nil) + assert_left_joins(5) { developers.to_sql } + assert_queries(2) { developers.to_a } assert_no_queries { developers.flat_map(&:projects).map(&:name) } assert_no_queries { developers.flat_map(&:projects).flat_map(&:developers).map(&:name) } end From 3678b281d8a6903c93bd5fd6810a3ebfe4773d3b Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Tue, 14 Mar 2023 20:49:42 +0300 Subject: [PATCH 6/7] Use a really nice sounding aliases --- activerecord/lib/active_record/relation/includes_tracker.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/relation/includes_tracker.rb b/activerecord/lib/active_record/relation/includes_tracker.rb index 4423ad76b286b..22179c2d9074a 100644 --- a/activerecord/lib/active_record/relation/includes_tracker.rb +++ b/activerecord/lib/active_record/relation/includes_tracker.rb @@ -4,11 +4,11 @@ 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(:present?) + select_includes_values_with_references(:any?) end def includes_values_non_referenced - select_includes_values_with_references(:blank?) + select_includes_values_with_references(:none?) end private From d8585ae0daccb786cb4accea4d4ac13c664a609c Mon Sep 17 00:00:00 2001 From: Artur Petrov Date: Tue, 14 Mar 2023 21:31:01 +0300 Subject: [PATCH 7/7] Improve README --- activerecord/CHANGELOG.md | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 7030ccaf9cd2e..63cbf735f128c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,33 +1,37 @@ * 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 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(:books, :post).to_a + > Author.includes(:post, books: :essay).to_a - Author Load (0.1ms) SELECT "authors".* FROM "authors" + 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]] - 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" IN (?, ?, ?, ?) [["book_id", 1], ["book_id", 3], ["book_id", 2], ["book_id", 4]] - > Author.includes(:books, :post).where(books: { language: 'EN' }).to_a + > 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, "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, "posts"."id" AS t2_r0, "posts"."author_id" AS t2_r1, "posts"."team_id" AS t2_r2, "posts"."title" AS t2_r3, "posts"."body" AS t2_r4, "posts"."type" AS t2_r5, "posts"."legacy_comments_count" AS t2_r6, "posts"."taggings_with_delete_all_count" AS t2_r7, "posts"."taggings_with_destroy_count" AS t2_r8, "posts"."tags_count" AS t2_r9, "posts"."indestructible_tags_count" AS t2_r10, "posts"."tags_with_destroy_count" AS t2_r11, "posts"."tags_with_nullify_count" AS t2_r12 FROM "authors" LEFT OUTER JOIN "books" ON "books"."author_id" = "authors"."id" LEFT OUTER JOIN "posts" ON "posts"."author_id" = "authors"."id" WHERE "books"."language" IS NULL + 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(:books, :post).where(books: { language: 'EN' }).to_a + > 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", 2], ["author_id", 3]] + 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]] ``` - *Artur Petrov* + It avoids joining everything which could be performance-heavy. + + *Artur Petrov, Svyatoslav Kryukov, Vladimir Dementyev* * Remove deprecated `Tasks::DatabaseTasks.schema_file_type`.