-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
support ANY()
op
#11849
support ANY()
op
#11849
Conversation
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.
Thank you @samuelcolvin -- this looks quite good to me
The only thing I think it needs is a .slt level test
Can you please add one?
Here are the instructions: https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest
Ideally you should be able to extend one of the existing test files in https://github.com/apache/datafusion/tree/main/datafusion/sqllogictest/test_files
Maybe somewhere in
datafusion/datafusion/sqllogictest/test_files/array.slt
Lines 4973 to 4979 in bddb641
## array_has/array_has_all/array_has_any | |
query BB | |
select array_has([], null), | |
array_has([1, 2, 3], null); | |
---- | |
false false |
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 think this is ready.
@@ -102,7 +102,7 @@ impl ExprPlanner for NestedFunctionPlanner { | |||
|
|||
fn plan_make_map(&self, args: Vec<Expr>) -> Result<PlannerResult<Vec<Expr>>> { | |||
if args.len() % 2 != 0 { | |||
return exec_err!("make_map requires an even number of arguments"); | |||
return plan_err!("make_map requires an even number of arguments"); |
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.
drive-by change, since this is a "plan" method I assume a "plan error" is more appropriate?
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.
Thank you @samuelcolvin -- looks great to me
Which issue does this PR close?
Half of #6602 - the overwhelmingly common half.
Rationale for this change
Trying to support as much postgreSQL syntax as possible,
ANY()
queries are fairly common.What changes are included in this PR?
ANY()
operator, as in'needle'=ANY(haystack)
array_has
to be less confusingANY()
Are these changes tested?
Yes, I can add more if you want.
Are there any user-facing changes?
Common queries with
ANY()
should now work