-
Notifications
You must be signed in to change notification settings - Fork 12
Better detection of standard preload #160
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
Conversation
4ca1bc3
to
bf2af28
Compare
update:
|
bf2af28
to
17cb22d
Compare
17cb22d
to
a37b1df
Compare
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
1 similar comment
This pull request has been automatically marked as stale because it has not been updated for at least 3 months. If these changes are still valid, please remove the |
a37b1df
to
173049d
Compare
173049d
to
87358d8
Compare
Kicking so it tests with rails 7.2 |
87358d8
to
5cd9dee
Compare
This ends up being an optimization. But when preloading `[:association]`, it runs a separate preloader on `:association`. This skips the interim step
5cd9dee
to
36bbc38
Compare
Checked commit kbrock@36bbc38 with ruby 3.1.5, rubocop 1.56.3, haml-lint 0.62.0, and yamllint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This ends up being an optimization.
Before
First off,
includes(:association)
hasincludes_values = [:association]
, or theuses
clause stores an array.When the framework uses the
ActiveRecord::Associations::Preloader
, the associations are passed in. If the associations go in via a single association (e.g.::books
), then ourPreloader#call
patch detects this and callssuper
right away.But since
includes(:association)
storesincludes_values
as an array (e.g.:[:association]
) oruses
as an array, we often call thePreloader.call
with the array of associations.This second scenario, creates an extra preloader, then group_by the records creating a second array. It is this extra preloading, grouping, and array that is avoided.
In our tests, this does not happen often, but when running miq tests, this case seemed to happen often.
After
Preload#call
will also detect if the association is an array with a single symbol. The extra processing is avoided.