Merge pull request #48785 from reid-rigo/strict-loading-n-plus-one-on…

…ly-eager-loading-fix

Fix child association loading in `:n_plus_one_only` mode

activerecord/CHANGELOG.md

@@ -1,3 +1,33 @@
+*   Strict loading using `:n_plus_one_only` does not eagerly load child associations.
+
+    With this change, child associations are no longer eagerly loaded, to
+    match intended behavior and to prevent non-deterministic order issues caused
+    by calling methods like `first` or `last`. As `first` and `last` don't cause
+    an N+1 by themselves, calling child associations will no longer raise.
+    Fixes #49473.
+
+    Before:
+
+    ```ruby
+    person = Person.find(1)
+    person.strict_loading!(mode: :n_plus_one_only)
+    person.posts.first
+    # SELECT * FROM posts WHERE person_id = 1; -- non-deterministic order
+    person.posts.first.firm # raises ActiveRecord::StrictLoadingViolationError
+    ```
+
+    After:
+
+    ```ruby
+    person = Person.find(1)
+    person.strict_loading!(mode: :n_plus_one_only)
+    person.posts.first # this is 1+1, not N+1
+    # SELECT * FROM posts WHERE person_id = 1 ORDER BY id LIMIT 1;
+    person.posts.first.firm # no longer raises
+    ```
+
+    *Reid Lynch*
+
 *   Using `Model.query_constraints` with a single non-primary-key column used to raise as expected, but with an
     incorrect error message. This has been fixed to raise with a more appropriate error message.
 

activerecord/lib/active_record/associations/collection_association.rb

@@ -303,7 +303,7 @@ def null_scope?
 
       def find_from_target?
         loaded? ||
-          owner.strict_loading? ||
+          (owner.strict_loading? && owner.strict_loading_all?) ||
           reflection.strict_loading? ||
           owner.new_record? ||
           target.any? { |record| record.new_record? || record.changed? }

activerecord/lib/active_record/core.rb

@@ -701,6 +701,11 @@ def strict_loading_n_plus_one_only?
       @strict_loading_mode == :n_plus_one_only
     end
 
+    # Returns +true+ if the record uses strict_loading with +:all+ mode enabled.
+    def strict_loading_all?
+      @strict_loading_mode == :all
+    end
+
     # Marks this record as read only.
     #
     #   customer = Customer.first

activerecord/test/cases/strict_loading_test.rb

@@ -78,6 +78,18 @@ def test_strict_loading_n_plus_one_only_mode
     end
   end
 
+  def test_strict_loading_n_plus_one_only_mode_does_not_eager_load_child_associations
+    developer = Developer.first
+    developer.strict_loading!(mode: :n_plus_one_only)
+    developer.projects.first
+
+    assert_not_predicate developer.projects, :loaded?
+
+    assert_nothing_raised do
+      developer.projects.first.firm
+    end
+  end
+
   def test_strict_loading
     Developer.all.each { |d| assert_not d.strict_loading? }
     Developer.strict_loading.each { |d| assert d.strict_loading? }