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 PostgreSQL array subquery constructor #566
Changes from all commits
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 |
---|---|---|
|
@@ -2597,7 +2597,7 @@ fn parse_bad_constraint() { | |
|
||
#[test] | ||
fn parse_scalar_function_in_projection() { | ||
let names = vec!["sqrt", "array", "foo"]; | ||
let names = vec!["sqrt", "foo"]; | ||
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 may be a controversial change for this test. PostgreSQL doesn't have a function-based array constructor ( 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 test was added in #432 Can you review the description on that PR and let me know what you think? 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. Interestingly, postgres doesn't allow the syntax used in the DataFusion test (and referred to in #432) alamb=# SELECT array(c1, cast(c2 as varchar)) FROM test;
ERROR: syntax error at or near "c1"
LINE 1: SELECT array(c1, cast(c2 as varchar)) FROM test;
^ https://github.com/apache/arrow-datafusion/blame/834924f274d9e04444030b8eb3e07e1035fcd3cf/datafusion/core/tests/sql/functions.rs#L134 doesn't really help me figure out from who the test originated from as it was moved (maybe some more poking around could find the original author) @ovr do you remember any of the previous history here? 🤔 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 poked around and found this commit: apache/datafusion@98b710a 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.
Nice -- that came in from apache/arrow#8102 / https://issues.apache.org/jira/browse/ARROW-9902 by @jorgecarleitao and approved by @andygrove Can either if you remember if/how using 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 support this as long as DataFusion team's okay with that. I haven't been able to find a dialect with built-in 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. @alamb do you feel it's a good idea 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.
Here is the spark reference docs: https://spark.apache.org/docs/2.4.1/api/sql/index.html#array (though perhaps you mean there is no
Sounds reasonable to me -- can you also possibly link one or two examples from other dialects that support the array subquery constructor? 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. PostgreSQL array subquery constructor (in fact, all of array constructors): https://www.postgresql.org/docs/current/sql-expressions.html#SQL-SYNTAX-ARRAY-CONSTRUCTORS I haven't been able to find info on other dialects present in the crate to support array subquery, but at least CockroachDB and BigQuery support array subquery. However while searching around I found that ClickHouse (which we do have a dialect for) actually has array function constructor built in: https://clickhouse.com/docs/en/sql-reference/functions/array-functions/#arrayx1--operator-x1- So we can either make a decision for leaving this on for specific dialects ( 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. Let's negate it for |
||
|
||
for function_name in names { | ||
// like SELECT sqrt(id) FROM foo | ||
|
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.
👌 Very nice @MazterQyou