Skip to content

Fix overlap lookup in subqueries. #5

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

Merged

Conversation

WaVEV
Copy link
Collaborator

@WaVEV WaVEV commented Jan 8, 2025

fixes mongodb#209

This PR could be after arrayfield is merged.

@WaVEV WaVEV requested a review from timgraham January 8, 2025 04:33
@timgraham
Copy link
Owner

What do you think about a follow-up refactor that adds get_subquery_wrapping_pipeline() to the in/range/overlap lookup classes so these wrappers can be declared on the lookup rather than hardcoding/special casing lookup names in query(). It seems that would be a better separation of concerns.

@WaVEV
Copy link
Collaborator Author

WaVEV commented Jan 8, 2025 via email

@WaVEV
Copy link
Collaborator Author

WaVEV commented Jan 8, 2025

What do you think about a follow-up refactor that adds get_subquery_wrapping_pipeline() to the in/range/overlap lookup classes so these wrappers can be declared on the lookup rather than hardcoding/special casing lookup names in query(). It seems that would be a better separation of concerns.

🤔 the idea is to call this getter in the Query.as_mql or in the in/range/overlaps/contains (yes this one will be added 😬 )
Or as a post process in in/range/overlaps/contains as_mql method?

@timgraham
Copy link
Owner

The first idea!

@WaVEV WaVEV force-pushed the arrayfield-fix-query-with-subqueries branch 2 times, most recently from 4ecc594 to 4ea54df Compare January 8, 2025 23:38
@timgraham timgraham force-pushed the arrayfield-fix-query-with-subqueries branch from 4ea54df to a769278 Compare January 9, 2025 02:45
@timgraham timgraham merged commit a769278 into timgraham:arrayfield Jan 9, 2025
@WaVEV WaVEV deleted the arrayfield-fix-query-with-subqueries branch January 9, 2025 18:11
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.

2 participants