Repository navigation
Conversation
…ule-relative class_name preload matches its work queue against assoc_map, whose keys are inverse_class_name values (resolved class names). The queue was fed the raw class_name option instead, so a class_name like "::Photo" or a module-relative "Item" never matched. The nested association was then silently skipped and loaded with one query per document. Push the resolved class name, keeping the raw class_name only for polymorphic associations where no single class is resolved.
jamis
requested review from
comandeo-mongo
and
a balanced review from Copilot
October 26, 2026 22:30
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The polymorphic guard still skips resolvable as: associations, and fixture teardown leaves temporary models registered.
2 open issues
What changed in this PR
This PR addresses silently skipped nested includes in Mongoid when an association’s class_name uses a leading :: or a module-relative name.
Changes:
- Queues resolved target class names for non-polymorphic associations.
- Adds regression specs checking that nested associations load without extra queries for both naming forms.
| File | Description |
|---|---|
spec/mongoid/criteria/includable_spec.rb |
Adds regression fixtures and zero-extra-query assertions. |
lib/mongoid/association/eager_loadable.rb |
Uses resolved class names when queuing nested preloads. |
Review effort: Balanced
…iations Resolve the target class name when queuing nested preloads rather than falling back to the raw class_name for every polymorphic association. Parent-side `as:` associations report polymorphic? true but have a fixed, resolvable target, so their nested includes were skipped (N+1) when the class_name was ::-prefixed or module-relative. Also deregister the models defined in includable_spec.rb so they no longer leak into Mongoid.models after each context.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nested includes were being silently dropped when the parent association's
class_namewasn't the resolved class name — either a"::"-prefixed string likeclass_name: "::Photo", or a module-relative one likeclass_name: "Item"inside a module. The parent association still eager-loaded, but the nested one didn't, so you'd get an N+1 with no error raised.The cause is in
preload: the work queue is matched againstassoc_mapkeys, which areinverse_class_namevalues (resolved names like"Photo"or"Shop::Item"), but the queue was fed the rawclass_nameoption. Those never matched, so the nested association was skipped entirely.The fix pushes the resolved class name instead:
Polymorphic associations keep the raw
class_name, since they don't resolve to a single class.