-
Notifications
You must be signed in to change notification settings - Fork 29
INTPYTHON-750: Support converting $expr with $getField paths #392
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?
Changes from all commits
1da1bc1
0c27833
22a43d4
edc0746
088ffb2
cfd13b6
7ab6b37
07e6ffb
bdc6ae0
4257582
a88fbe7
e62e299
00426a7
8331701
77f9d34
f783c8e
797da9e
b8c99ca
db17c7a
aacdd4d
49c34d6
6a14b1d
377f9ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -213,3 +213,49 @@ def test_deeply_nested_logical_operator_with_variable(self): | |
} | ||
] | ||
self.assertOptimizerEqual(expr, expected) | ||
|
||
def test_getfield_usage_on_dual_binary_operator(self): | ||
expr = { | ||
"$expr": { | ||
"$gt": [ | ||
{"$getField": {"input": "$price", "field": "value"}}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we optimize this down to remove the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could. I'm trying to think if there's any case where this would be an issue if it mistakenly resolved it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would have to be a two-phase conversion. I think it may even be best to still change it in our actual query code. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm assuming here that this change ends up being needed. Holding off until we either get customer complaints or the refactor can't solve this makes more sense to me for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I tried to make it work (and I think I managed to), but the silent mutation of |
||
{"$getField": {"input": "$discounted_price", "field": "value"}}, | ||
] | ||
} | ||
} | ||
expected = [ | ||
{ | ||
"$match": { | ||
"$expr": { | ||
"$gt": [ | ||
{"$getField": {"input": "$price", "field": "value"}}, | ||
{"$getField": {"input": "$discounted_price", "field": "value"}}, | ||
] | ||
} | ||
} | ||
} | ||
] | ||
self.assertOptimizerEqual(expr, expected) | ||
|
||
def test_getfield_usage_on_onesided_binary_operator(self): | ||
expr = {"$expr": {"$gt": [{"$getField": {"input": "$price", "field": "value"}}, 100]}} | ||
# This should create a proper match condition with no $expr | ||
expected = [{"$match": {"price.value": {"$gt": 100}}}] | ||
self.assertOptimizerEqual(expr, expected) | ||
|
||
def test_nested_getfield_usage_on_onesided_binary(self): | ||
expr = { | ||
"$expr": { | ||
"$gt": [ | ||
{ | ||
"$getField": { | ||
"input": {"$getField": {"input": "$item", "field": "price"}}, | ||
"field": "value", | ||
} | ||
}, | ||
100, | ||
] | ||
} | ||
} | ||
expected = [{"$match": {"item.price.value": {"$gt": 100}}}] | ||
self.assertOptimizerEqual(expr, expected) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use explicit tuple syntax
(list, tuple, set)
instead of union operator|
for better compatibility with older Python versions.Copilot uses AI. Check for mistakes.