Skip to content

Conversation

@jamis
Copy link
Contributor

@jamis jamis commented Nov 19, 2025

This addresses a few bugs with the implementation of #pluck on associations.

  1. It now respects localized fields, and the current localization settings.
  2. It now plucks loaded and unloaded records from the association.
  3. It now does the right thing when fields are referenced by an alias.

Tests from #6044 (from @johnnyshields) are included in this PR (thank you, Johnny!).

Copilot AI review requested due to automatic review settings November 19, 2025 16:42
@jamis jamis requested a review from a team as a code owner November 19, 2025 16:42
@jamis jamis requested a review from comandeo-mongo November 19, 2025 16:42
Copilot finished reviewing on behalf of jamis November 19, 2025 16:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes several bugs in the #pluck implementation for associations in Mongoid. It adds proper support for localized fields (respecting current locale and fallbacks), handles both loaded and unloaded association records correctly, and properly resolves field aliases.

Key changes:

  • Extracts common pluck functionality into a new Pluckable module for code reuse
  • Updates has_many and embeds_many associations to properly handle plucking from mixed loaded/unloaded/added documents
  • Adds comprehensive test coverage for localized fields, aliased fields, and embedded documents

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
lib/mongoid/pluckable.rb New shared module providing pluck functionality with support for field aliases, localization, and demongoization
lib/mongoid/contextual/mongo.rb Refactored to use the new Pluckable module, removing duplicated code
lib/mongoid/association/referenced/has_many/enumerable.rb Added pluck implementation that handles loaded, unloaded, and added documents
lib/mongoid/association/referenced/has_many/proxy.rb Added pluck delegation to the enumerable target
lib/mongoid/association/embedded/embeds_many/proxy.rb Added pluck delegation to criteria alongside existing find delegation
spec/support/models/*.rb Added test models with localized and aliased fields to support comprehensive testing
spec/mongoid/association/referenced/has_many/enumerable_spec.rb Added extensive test coverage for pluck functionality including localization, aliases, and embedded documents

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jamis jamis changed the title MONGOID-5901 Fix #pluck on associations MONGOID-5900, MONGOID-5901 Fix #pluck on associations Nov 19, 2025
@jamis jamis added the bug Fixes a bug, with no new features or broken compatibility label Nov 19, 2025
@johnnyshields
Copy link
Contributor

johnnyshields commented Nov 20, 2025

Hi Jamis, great, thank you for handling this.

I noticed you extracted out a Pluckable concern. After merging this PR, I would encourage you to go a step further and consider merging my PluckEnumerator and #pluck_each PR here: #5497

PluckEnumerator is a big improvement, not only in terms of code organization, but also behavior. Currently, #pluck force-loads the plucked values for all documents in the query. So if you are pluck-ing 1,000,000 docs, you need to load all 1,000,000 values into Ruby's memory, which will cause an OOM crash of your app. #pluck_each behaves like Mongoid's #each iterator--it loads the plucks in batches and GC's them. I use it in dozens of places in my app's code.

@jamis jamis merged commit 741ae8b into mongodb:master Nov 21, 2025
63 checks passed
@jamis jamis deleted the 5901-pluck-localized-aliased branch November 21, 2025 15:46
jamis added a commit to jamis/mongoid that referenced this pull request Nov 21, 2025
* use the specialized implementation of pluck

* use a custom pluck implementation

* new file

* incorporate Johnny's changes from mongodb#6044

* use same submodules as master

* Make changes suggested by copilot review

* rubocop appeasement

* fix broken refactoring
jamis added a commit that referenced this pull request Nov 24, 2025
* use the specialized implementation of pluck

* use a custom pluck implementation

* new file

* incorporate Johnny's changes from #6044

* use same submodules as master

* Make changes suggested by copilot review

* rubocop appeasement

* fix broken refactoring
@jamis jamis changed the title MONGOID-5900, MONGOID-5901 Fix #pluck on associations MONGOID-5900 Fix #pluck on associations Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Fixes a bug, with no new features or broken compatibility

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants