-
Notifications
You must be signed in to change notification settings - Fork 10
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 |
jrafanie
left a comment
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 theusesclause 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#callpatch detects this and callssuperright away.But since
includes(:association)storesincludes_valuesas an array (e.g.:[:association]) orusesas an array, we often call thePreloader.callwith 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#callwill also detect if the association is an array with a single symbol. The extra processing is avoided.