-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
include some BinaryOperator from sqlparser #15327
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Ruihang Xia <[email protected]>
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.
Thanks @waynexia -- I think the idea is great. However, I think there should be sql level tests (sqllogitests) that run these operators. I think it will make extending operators easier for other systems built on DataFusion
The tests can / should probably error with unsupported
It might also be a good idea to include some documentation in the operators themselves that DataFusion doesn't have default implementations
AtArrow | ArrowAt | Arrow | LongArrow | HashArrow | HashLongArrow | AtAt | ||
| HashMinus | AtQuestion | Question | QuestionAnd | QuestionPipe | ||
| IntegerDivide => { | ||
unreachable!("These operators should be rewritten to functions") |
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.
I dont think this code is unreachable -- I got it to panic:
DataFusion CLI v46.0.1
> select 'foo' @> 'bar';
thread 'main' panicked at datafusion/physical-expr/src/expressions/binary.rs:799:17:
internal error: entered unreachable code: These operators should be rewritten to functions
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
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.
I just realized <@
and @>
are only supported for lists 🤣
Signed-off-by: Ruihang Xia <[email protected]>
That's a good idea! I think I know them much better after writing some SQLs (though none of them can run... lol) |
Signed-off-by: Ruihang Xia <[email protected]>
Added in 5828cba 👍 |
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.
THanks @waynexia
Which issue does this PR close?
Rationale for this change
Follows #15326, this patch includes
Arrow
,LongArrow
,HashArrow
,HashLongArrow
,AtAt
,IntegerDivide
,HashMinus
,AtQuestion
,Question
,QuestionAnd
andQuestionPipe
binary operators.What changes are included in this PR?
New operators. Only
XOR
is actually implemented (functional) among them, by mapping to existingBitwiseXor
.Are these changes tested?
Compiler tests it
Are there any user-facing changes?
New APIs