-
Notifications
You must be signed in to change notification settings - Fork 30
INTPYTHON-824 Refactor $lookup stages to use localField and foreignField #443
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
base: main
Are you sure you want to change the base?
Conversation
c57b03f to
8330b8f
Compare
|
@WaVEV Awesome! I merged this with the QE PR for testing. With a separate encrypted connection, I get these 2 errors: With a single encrypted connection I get these errors: |
|
The first two errors about "DatabaseError not raised" demonstrate queries that couldn't run before this change but now no longer use $let. I'll have to add new tests with queries that still do. The error "Pipelines in updates are not allowed to modify encrypted fields or add fields which are not present in the schema" suggests a query that uses |
INTPYTHON-824
Summary
This PR refactors the generation of
$lookupstages to use the simplerlocalField/foreignFieldsyntax when possible.Previously, all joins were built using the
letandpipelineform, even for simple one-field equality joins. This change improves readability and allows MongoDB to optimize execution internally for straightforward joins, while still supporting additional filtering logic throughletvariables when necessary.Changes in this PR
Added logic to detect when a
$lookupstage can safely uselocalFieldandforeignFieldinstead ofletandpipeline.Updated the query compiler to:
localField/foreignFieldwhen the join condition involves a single equality comparison between plain fields.localField/foreignFieldwith alet/pipelinewhen additional filter expressions are required beyond the join condition.Ensured compatibility with existing
$lookupstages in subqueries and nested joins.Added test coverage to confirm behavior parity between both forms.
Test Plan
Added new unit tests verifying that:
$lookupusinglocalField/foreignField.let/pipelineform.Verified all existing tests pass, ensuring backward compatibility.
Manual validation with sample queries confirmed that results remain identical before and after the refactor.
Checklist
Checklist for Author
Checklist for Reviewer @Jibola @aclark4life @timgraham
Focus Areas for Reviewer
$lookupcases (using bothlocalFieldandlet) are generated correctly and still behave as expected.$lookupscenarios, including nested and subquery joins, remain fully supported.