Skip to content
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

fix counter preloading #12

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

egor-lukin
Copy link
Contributor

@egor-lukin egor-lukin commented Mar 28, 2023

What is the purpose of this pull request?

Fix preloading counters for multiple records.
Fixes #11

What changes did you make?

  • Removed id param from associated_records scope. It brakes the Rails preload mechanism (and it's useless in general).
  • Refactored the read_slotted_counter method. We don't need to do branching because load_target does the same - loads from db if there are no cached records and loads from memory if there are.

Checklist

  • I've added tests for this change
  • I've added a Changelog entry

@egor-lukin egor-lukin force-pushed the fix/counter_preloading branch from f685986 to 1c80967 Compare March 28, 2023 19:28
@egor-lukin egor-lukin marked this pull request as ready for review March 28, 2023 19:31
@egor-lukin egor-lukin requested a review from palkan March 28, 2023 19:31
Copy link
Member

@palkan palkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT 👍

@egor-lukin egor-lukin merged commit 41f5fc1 into evilmartians:master Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

N+1 query when using with_slotted_counters()
2 participants