NOT operator not return internal error when args are not boolean value #8982
Conversation
github-actions
bot
added
physical-expr
There was a problem hiding this comment.
Thank you @guojidan look great to me! I think you can merge main to resolve the conflicts.
There was a problem hiding this comment.
I agree with @haohuaijin -- very nice @guojidan . Thank you 🙏
I think this PR just needs to merge up from main and resolve the conflict and it will be good to go.
Thank you for the string of excellent PRs @guojidan
| Expr::Not(expr) => { | ||
| let expr = not(get_casted_expr_for_bool_op(&expr, &self.schema)?); | ||
| Ok(expr) | ||
| } | ||
| Expr::IsTrue(expr) => { |
There was a problem hiding this comment.
It might make sense to remove the catch all from this match expression
And explicitly list out all Expr types so that we don't accidentally another Expr type in the future
Is that something you might be willing to do in another PR @guojidan ?
|
cc @alamb need rerun test case, look like network error |
restarted! BTW another trick I think works is closing and the reopening the PR |
|
Thanks again @guojidan ! |
Which issue does this PR close?
Closes #8965.
Rationale for this change
What changes are included in this PR?
in
TypeCoercionRewriterreturn an error if the argument to not can't be coerced to BooleanAre these changes tested?
Are there any user-facing changes?