Expression: Part of the expression is ignored when multiple and/or expressions are specified #65
Conversation
pyiceberg/expressions/parser.py
Outdated
| if len(result[0]) > 2: | ||
| return And(result[0][0], result[0][1], *result[0][2:]) | ||
| return And(result[0][0], result[0][1]) |
There was a problem hiding this comment.
Uh maybe not the most pythonic...I guess a ternary which sets the *args option if it's > 2 otherwise AlwaysTrue (for And) and then for Or would be AlwaysFalse so it just considers the first 2.
There was a problem hiding this comment.
I realized I can just pass in *result[0]
|
This is a serious one, thanks for reporting @puchengy |
amogh-jahagirdar
force-pushed
the
python-fix-multiple-and-or
branch
from
a0db070 to
4c2c1af
Compare
…a row filter string
amogh-jahagirdar
force-pushed
the
python-fix-multiple-and-or
branch
from
4c2c1af to
3c0d80e
Compare
|
Yes thanks @puchengy for reporting this, please also take a look a this fix when you get a chance! |
|
@amogh-jahagirdar The test result LGTM. However, I am not familiar with the implementation, please feel free to go ahead and merge. |
| @@ -233,11 +233,11 @@ def handle_not(result: ParseResults) -> Not: | |||
|
|
|||
|
|
|||
| def handle_and(result: ParseResults) -> And: | |||
| return And(result[0][0], result[0][1]) | |||
| return And(*result[0]) | |||
There was a problem hiding this comment.
Why does the parser produce more than 2 child expressions?
Is this a result of using infix_notation that I didn't realize at the time?
|
Thanks for jumping on the fix, @amogh-jahagirdar! @Fokko, should we release 0.5.1 with this patch? |
Yes, I think that's a good idea |
Fokko
changed the title
Fixes #64 . If there are multiple and/or conditions currently, our expression parser will ignore anything after the second predicate. This change fixes the issue by forwarding the remaining predicates as the argument to
*rest: BooleanExpressionfor And and Or.