ActiveRecord#includes gives different results if the includes order is different.

[github0013]

Steps to reproduce

I prepared a repo.
https://github.com/github0013/activerecord-includes

Expected behavior

Switching the order in #includes should not change the results. (please see the repo)

• A.includes(:c_type_xs, :ds).find(subject.id).c_type_xs.size # => 1
• A.includes(:ds, :c_type_xs).find(subject.id).c_type_xs.size # => 1

Actual behavior

• A.includes(:c_type_xs, :ds).find(subject.id).c_type_xs.size # => 1
• A.includes(:ds, :c_type_xs).find(subject.id).c_type_xs.size # => 4

System configuration

Rails version:

• 5.2.4.1
• 6.1.1

Ruby version:

• ruby 2.7.1p83 (2020-03-31 revision a0c7c23c9c) [x86_64-linux-musl]

[intrip]

I do confirm it's an issue, here you find a simple reproduction script (FF to add it to the issue description if you like).
The issue is with .preload, which is triggered by includes in such operation. Explicitly using eager_load solves the problem.

I didn't see any existing PRs for this: I'll work on a fix.
/cc @kamipo

[intrip]

I've narrowed a bit the issue and I think it's related to the memoizing in

rails/activerecord/lib/active_record/associations/preloader/association.rb

Line 25 in 0f09dfc

@already_loaded = owners.all? { |o| o.association(reflection.name).loaded? }

Which doesn't work well when you have a scope in an has_many_through and the through_relation has already been preloaded without using the scope.

I'll try to figure out how to improve this check.

[intrip]

@kamipo #41489 fixes this issue, could you be so kind to take a look at it?
/cc @github0013

[github0013]

gem 'rails', git: "https://github.com/intrip/rails.git", branch: "41378-fix-preload-scoped"

Thanks a million!
I tried your branch, and my rspec is all green!
Just wondering if this same fix will be applied to version 5 and 6 because it shows this rails is Rails 7.0.0.alpha.

[intrip]

You're welcome!
From the mainteinance policy looks ike if the PR is accepted it may be ported to 6.1 max

[intrip]

@github0013 could you please reopen? I'd wait for the PR to be accepted/merged before closing the issue 🙂

[rails-bot]

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 6-1-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

[github0013]

any updates?

[intrip]

#41489 is still waiting for review sorry

[Reedlee]

hello there,

We also spotted this issue in rails 6.0.3.6 and we found the next workarounds. I know that you already prepared a fix which we can use a patch but might be it helpful:

class A < ApplicationRecord
  has_many :bs
  has_many :__bs # by using it you can avoid this issue 
  has_many :cs, through: :bs, source: "cs"
  has_many :c_type_xs, -> { type_x }, through: :__bs, source: "cs"
end

[github0013]

@Reedlee __ + association name is like a hidden private instance or something?
I am trying to figure out why you can specify __bs as an association name without giving class_name attribute.

[intrip]

The reason it works is because going through __bs uses a different cache, you can find more details in the PR attached.

[Reedlee]

@Reedlee __ + association name is like a hidden private instance or something? I am trying to figure out why you can specify __bs as an association name without giving class_name attribute.

Hello @github0013
It's my bad :) I forgot about adding class_name attribute into the example. So it should be:

class A < ApplicationRecord
  has_many :bs
  has_many :__bs, class_name: "B" # by using it you can avoid this issue. Also, you can use any name instead of __bs 
  has_many :cs, through: :bs, source: "cs"
  has_many :c_type_xs, -> { type_x }, through: :__bs, source: "cs"
end

Hello @intrip
Thank you for your PR. I hope it will be merged soon:)
By the way, I don't know if it matters for your PR. I checked this workaround for eager_load method and it didn't work. It's just for your information that it's good to check eager_load also. :)

[intrip]

@Reedlee If I recall correctly eager_load is not affected by this issue, can you please verify?

[Reedlee]

@intrip hey,

Sorry for the late response.
I checked eager_load in the github0013/activerecord-includes#1:

🤔 It's interesting that this problem with order exists for 5.2.x, 6.0.x. but 6.1.x doesn't have.

Also important to say that the problem with ordering exists if we are trying to get value from cs by filtering them by using bs, please, take a look at c_of_by method.

class A < ApplicationRecord
  has_many :bs
  has_many :cs, through: :bs, source: "cs"

# doesn't have the problem related to ordering for eager_load
  has_many :c_type_xs, -> { type_x }, through: :bs, source: "cs"

# has the problem related to ordering for eager_load
  has_many :c_of_by, -> { where(bs: {name: 'y'}) }, through: :bs, source: "cs"
end

So if we are running cases like:
https://github.com/github0013/activerecord-includes/pull/1/files#diff-f411c2d0fb2322c513f7d9588080a227ef984fd838808d5665962a0ba82d180eR89-R92