You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2024/05/06 14:35:05 UTC
[PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
tisonkun opened a new pull request, #10392:
URL: https://github.com/apache/datafusion/pull/10392
Upgrade sqlparser to 0.46.0. This should handle a few of breaking changes:
* [x] Insert and Delete are now encapsulated in a struct.
* [ ] JsonAccess refactor - https://github.com/sqlparser-rs/sqlparser-rs/pull/1215/
* [ ] SQLFunction refactor - https://github.com/sqlparser-rs/sqlparser-rs/pull/1247
This is working in progress.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1600648484
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -895,111 +844,75 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
let args = vec![fullstr, substr];
Ok(Expr::ScalarFunction(ScalarFunction::new_udf(fun, args)))
}
- fn sql_agg_with_filter_to_expr(
Review Comment:
there is no special sqlparser struct for this kind of aggregate function anymore -- it is all unified
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2152294013
I did a final merge up from main to make sure we don't have any logical conflicts and then I plan to merge this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2140098341
I updated went and updated @jmhain 's PR https://github.com/sqlparser-rs/sqlparser-rs/pull/1290 to support the stride argument https://github.com/sqlparser-rs/sqlparser-rs/pull/1290#issuecomment-2139994033
I then pushed a commit to this branch to rework how field access is planned. Let's see how the CI does
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2138446665
Thanks @tisonkun -- I plan to review this and other sqlparser stuff tomorrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #10392:
URL: https://github.com/apache/datafusion/pull/10392
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1605946538
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
I guess it's because we remove the "UnQuotedString" and values is not a word?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1623272194
##########
.github/workflows/rust.yml:
##########
@@ -334,6 +334,8 @@ jobs:
run: |
export PATH=$PATH:$HOME/d/protoc/bin
cargo test --lib --tests --bins --features avro,json,backtrace
+ # 8MB stack size otherwise the cli_integration test fails
Review Comment:
👍 reverting the change and will try something else
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1619614590
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -215,11 +216,28 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
_ => (expr, false),
}
}
+
let expr =
- self.sql_expr_to_logical_expr(*obj, schema, planner_context)?;
+ self.sql_expr_to_logical_expr(*expr, schema, planner_context)?;
+
+ let indexes = match *subscript {
+ Subscript::Index { index } => vec![index],
+ Subscript::Slice { lower_bound, upper_bound } => {
+ let mut indexes = vec![];
+ if let Some(lower_bound) = lower_bound {
+ indexes.push(lower_bound);
+ }
+ if let Some(upper_bound) = upper_bound {
+ indexes.push(upper_bound);
+ }
+ indexes
+ }
+ };
Review Comment:
Also, the patch still fails to support `select make_array(1, 2, 3)[1:2:2], make_array(1.0, 2.0, 3.0)[2:3:2], make_array('h', 'e', 'l', 'l', 'o')[2:4:2];`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1626434949
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
I added a test and made this syntax work in this pr in 84468515808991159632aac8db0f68e7e039419b
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "jmhain (via GitHub)" <gi...@apache.org>.
jmhain commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1606080413
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
It's because of [this PR](https://github.com/sqlparser-rs/sqlparser-rs/pull/1249), which made `VALUES` allowed in a `FROM` without a subquery . I'll make a fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "jmhain (via GitHub)" <gi...@apache.org>.
jmhain commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1606080413
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
It's because of [this PR](https://github.com/sqlparser-rs/sqlparser-rs/pull/1249), which made `VALUES` allowed in a `FROM` without a subquery. I'll make a fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1599294528
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -943,11 +950,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
) => GetFieldAccess::NamedStructField {
name: ScalarValue::from(s),
},
- SQLExpr::JsonAccess {
- left,
- operator: JsonOperator::Colon,
- right,
- } => {
+ SQLExpr::JsonAccess { value, path } => {
Review Comment:
@alamb @jmhain, do you have ideas on how we should modify this branch now?
Previously, we handled `xxx::yyy` in this branch. But now, `JsonOperator::Colon` no longer exists.
It seems to me that we should handle `path.len() == 1` or `path.len() == 2` and build a `GetFieldAccess::ListRange` as the original logic?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1600615522
##########
datafusion/sql/src/expr/function.rs:
##########
@@ -90,13 +92,63 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
name,
args,
over,
- distinct,
filter,
- null_treatment,
- special: _, // true if not called with trailing parens
- order_by,
+ mut null_treatment,
+ within_group,
} = function;
+ // todo handle case when args is none (for current_timestamp)
+ let FunctionArguments::List(args) = args else {
+ return not_impl_err!("Unsupported function argument {args:?}");
+ };
+
+ let FunctionArgumentList {
Review Comment:
I took a shot at handling the changes to argument handling here (due to https://github.com/sqlparser-rs/sqlparser-rs/pull/1247)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2145223156
My plan for this PR is to disable the failing test on windows and file a ticket to investigate
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2100223608
Pushed a commit to handle other breaking changes. Now remains the JsonAccess and Agg func refactor. I'm still understanding how they are logically changed ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1605950326
##########
datafusion/sqllogictest/test_files/array.slt:
##########
Review Comment:
Can be a bug after the JSON path parse changes - https://github.com/sqlparser-rs/sqlparser-rs/pull/1215#issuecomment-2119108808
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1605947314
##########
datafusion/sqllogictest/test_files/array.slt:
##########
Review Comment:
New failure:
```
Running "array.slt"
External error: query failed: DataFusion error: SQL error: ParserError("Expected variant object key name, found: 2")
[SQL] select make_array(1, 2, 3)[1:2], make_array(1.0, 2.0, 3.0)[2:3], make_array('h', 'e', 'l', 'l', 'o')[2:4];
at test_files/array.slt:859
Error: Execution("1 failures")
```
Seems array slice syntax changed ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1607425124
##########
datafusion/sqllogictest/test_files/array.slt:
##########
Review Comment:
Thanks @tisonkun -- sounds like we should fix that upstream and then I can maybe make a new sqlparser release (or maybe I can make a minor patch release) 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2139572138
I have been playing around with this PR locally. I think we need to make some additional changes to how SQL planning works for these structured field accesses, but I think it will be simpler when we are done. I'll push some changes shortly
I also see what else is needed upstream in sqlparser and I'll try and add it to @jmhain 's branch too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143399298
Thanks @tisonkun -- I updated this PR to use the newly released 0.47.0 version of sqlparser-rs and updated the description. I suppose now we just need to debug the windows test (I have an idea) and we'll be good to go
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1623202665
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
Thanks for letting me know, @alamb
I just wonder if this change will diverge sqlparser from Postgres?
I guess function like this
```sql
CREATE FUNCTION strlen(name TEXT)
RETURNS int LANGUAGE plrust AS
$$
Ok(Some(name.unwrap().len() as i32))
$$;
```
Would not be possible without wrapping body with string
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2111110401
There is still a failure which I haven't had a chance to look into:
```shell
cargo test --test sqllogictests -- array
...
Running "array_query.slt"
Running "array.slt"
External error: query failed: DataFusion error: SQL error: ParserError("Expected variant object key name, found: 2")
[SQL] select make_array(1, 2, 3)[1:2], make_array(1.0, 2.0, 3.0)[2:3], make_array('h', 'e', 'l', 'l', 'o')[2:4];
at test_files/array.slt:859
Error: Execution("1 failures")
error: test failed, to rerun pass `-p datafusion-sqllogictest --test sqllogictests`
Caused by:
process didn't exit successfully: `/Users/andrewlamb/Software/datafusion/target/debug/deps/sqllogictests-b3a0a4f302be541c array` (exit status: 1)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2140423581
🎉 the tests are now passing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1617982950
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
Push a new commit to revert this work around. It's now fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2136212148
I've handled all other trivial new ASTs. But how to migrate `SQLExpr::ArrayIndex` to `SQLExpr::Subscript` still need some time to work out. Perhaps @jmhain can take a look and patching this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1625009837
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
It used to work, https://github.com/sqlparser-rs/sqlparser-rs/pull/1185
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1623224154
##########
.github/workflows/rust.yml:
##########
@@ -334,6 +334,8 @@ jobs:
run: |
export PATH=$PATH:$HOME/d/protoc/bin
cargo test --lib --tests --bins --features avro,json,backtrace
+ # 8MB stack size otherwise the cli_integration test fails
Review Comment:
Seems failure retains :<
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1623200250
##########
.github/workflows/rust.yml:
##########
@@ -334,6 +334,8 @@ jobs:
run: |
export PATH=$PATH:$HOME/d/protoc/bin
cargo test --lib --tests --bins --features avro,json,backtrace
+ # 8MB stack size otherwise the cli_integration test fails
Review Comment:
I added this to workaround the issue @tisonkun reports https://github.com/apache/datafusion/pull/10392#issuecomment-2143357574
If it passes CI I will file a ticket to track
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143383653
> Since we're on the sqlparser-rs main branch now, I'd consider this patch as ready to merge.
>
> But of course it's better to have a new release to sqlparser-rs and we move to that version. Hopefully it contains what it has now without new breaking changes :P
Thank you so much @tisonkun -- I agree
I will go make a sqlparser release now without any new changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1624642596
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
It seems like we may need to add `$$` string parsing support in sqlparser. I'll try and file a ticket later todday
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2098683236
@jmhain Thanks for your patch. I've applied it to this branch. Let us continue fixing the breaking changes here when we have time :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143342679
Pushed some commit to catch up new sqlparser-rs commits. Let's see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1600615522
##########
datafusion/sql/src/expr/function.rs:
##########
@@ -90,13 +92,63 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
name,
args,
over,
- distinct,
filter,
- null_treatment,
- special: _, // true if not called with trailing parens
- order_by,
+ mut null_treatment,
+ within_group,
} = function;
+ // todo handle case when args is none (for current_timestamp)
+ let FunctionArguments::List(args) = args else {
+ return not_impl_err!("Unsupported function argument {args:?}");
+ };
+
+ let FunctionArgumentList {
Review Comment:
I took a shot at handling the changes to argument handling here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143030177
I tried to update this PR to https://github.com/sqlparser-rs/sqlparser-rs/commit/afa5f08db9b1f3a4805f21fea6b1e72710cdb138 (the latest and greatest commit from sqlparser-rs) but the recent rework of the function definition seem to cause issues. I need to look at that again in more detail
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1605950326
##########
datafusion/sqllogictest/test_files/array.slt:
##########
Review Comment:
Can be a bug after the JSON path parse changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "jmhain (via GitHub)" <gi...@apache.org>.
jmhain commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1606080413
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
It's because of [this PR](https://github.com/sqlparser-rs/sqlparser-rs/pull/1249), which made `values` . I'll make a fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2136204241
I'm testing on @jmhain's branch on the subscript fixes and encounter the following new API breaks:
```
Compiling datafusion-sql v38.0.0 (/Users/tison/Brittani/arrow-datafusion/datafusion/sql)
error[E0599]: no variant named `ArrayIndex` found for enum `sqlparser::ast::Expr`
--> datafusion/sql/src/expr/mod.rs:197:22
|
197 | SQLExpr::ArrayIndex { obj, indexes } => {
| ^^^^^^^^^^ variant not found in `sqlparser::ast::Expr`
error[E0599]: no variant or associated item named `RawStringLiteral` found for enum `sqlparser::ast::Value` in the current scope
--> datafusion/sql/src/statement.rs:78:18
|
78 | | Value::RawStringLiteral(_)
| ^^^^^^^^^^^^^^^^
| |
| variant or associated item not found in `Value`
| help: there is a variant with a similar name: `HexStringLiteral`
error[E0026]: variant `sqlparser::ast::Statement::SetVariable` does not have a field named `variable`
--> datafusion/sql/src/statement.rs:232:17
|
232 | variable,
| ^^^^^^^^
| |
| variant `sqlparser::ast::Statement::SetVariable` does not have this field
| help: a field with a similar name exists: `variables`
Compiling datafusion-functions-aggregate v38.0.0 (/Users/tison/Brittani/arrow-datafusion/datafusion/functions-aggregate)
error[E0004]: non-exhaustive patterns: `FunctionArgumentClause::Having(_)` and `FunctionArgumentClause::Separator(_)` not covered
--> datafusion/sql/src/expr/function.rs:142:19
|
142 | match clause {
| ^^^^^^ patterns `FunctionArgumentClause::Having(_)` and `FunctionArgumentClause::Separator(_)` not covered
|
note: `FunctionArgumentClause` defined here
--> /Users/tison/.cargo/git/checkouts/sqlparser-rs-7df4902af8c87ae3/bd573b2/src/ast/mod.rs:4995:1
|
4995 | pub enum FunctionArgumentClause {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
5023 | Having(HavingBound),
| ------ not covered
...
5027 | Separator(Value),
| --------- not covered
= note: the matched value is of type `FunctionArgumentClause`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern, a match arm with multiple or-patterns as shown, or multiple match arms
|
166 ~ },
167 + FunctionArgumentClause::Having(_) | FunctionArgumentClause::Separator(_) => todo!()
|
```
Also related to -
* https://github.com/sqlparser-rs/sqlparser-rs/pull/1262
* https://github.com/sqlparser-rs/sqlparser-rs/pull/1252
* https://github.com/sqlparser-rs/sqlparser-rs/pull/1258
* https://github.com/sqlparser-rs/sqlparser-rs/pull/1256
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2136253325
Ensure all the sqllogictests passed without the array index cases:
<details><summary>Details</summary>
```
### Array index
## array[i]
# single index with scalars #1 (positive index)
query IRT
select make_array(1, 2, 3)[1], make_array(1.0, 2.0, 3.0)[2], make_array('h', 'e', 'l', 'l', 'o')[3];
----
1 2 l
# single index with scalars #2 (zero index)
query I
select make_array(1, 2, 3)[0];
----
NULL
# single index with scalars #3 (negative index)
query IRT
select make_array(1, 2, 3)[-1], make_array(1.0, 2.0, 3.0)[-2], make_array('h', 'e', 'l', 'l', 'o')[-3];
----
3 2 l
# single index with scalars #4 (complex index)
query IRT
select make_array(1, 2, 3)[1 + 2 - 1], make_array(1.0, 2.0, 3.0)[2 * 1 * 0 - 2], make_array('h', 'e', 'l', 'l', 'o')[2 - 3];
----
2 2 o
# single index with columns #1 (positive index)
query ?RT
select column1[2], column2[3], column3[1] from arrays;
----
[3, ] 3.3 L
[5, 6] 6.6 i
[7, 8] 9.9 d
[9, 10] 12.2 s
NULL 15.5 a
[13, 14] NULL ,
[, 18] 18.8 NULL
# single index with columns #2 (zero index)
query ?RT
select column1[0], column2[0], column3[0] from arrays;
----
NULL NULL NULL
NULL NULL NULL
NULL NULL NULL
NULL NULL NULL
NULL NULL NULL
NULL NULL NULL
NULL NULL NULL
# single index with columns #3 (negative index)
query ?RT
select column1[-2], column2[-3], column3[-1] from arrays;
----
[, 2] 1.1 m
[3, 4] NULL m
[5, 6] 7.7 r
[7, ] 10.1 t
NULL 13.3 t
[11, 12] NULL ,
[15, 16] 16.6 NULL
# single index with columns #4 (complex index)
query ?RT
select column1[9 - 7], column2[2 * 0], column3[1 - 3] from arrays;
----
[3, ] NULL e
[5, 6] NULL u
[7, 8] NULL o
[9, 10] NULL i
NULL NULL e
[13, 14] NULL NULL
[, 18] NULL NULL
# TODO: support index as column
# single index with columns #5 (index as column)
# query ?
# select make_array(1, 2, 3, 4, 5)[column2] from arrays_with_repeating_elements;
# ----
# TODO: support argument and index as columns
# single index with columns #6 (argument and index as columns)
# query I
# select column1[column2] from arrays_with_repeating_elements;
# ----
## array[i:j]
# multiple index with columns #1 (positive index)
query ???
select make_array(1, 2, 3)[1:2], make_array(1.0, 2.0, 3.0)[2:3], make_array('h', 'e', 'l', 'l', 'o')[2:4];
----
[1, 2] [2.0, 3.0] [e, l, l]
query ???
select arrow_cast([1, 2, 3], 'LargeList(Int64)')[1:2],
arrow_cast([1.0, 2.0, 3.0], 'LargeList(Int64)')[2:3],
arrow_cast(['h', 'e', 'l', 'l', 'o'], 'LargeList(Utf8)')[2:4]
;
----
[1, 2] [2, 3] [e, l, l]
# multiple index with columns #2 (zero index)
query ???
select make_array(1, 2, 3)[0:0], make_array(1.0, 2.0, 3.0)[0:2], make_array('h', 'e', 'l', 'l', 'o')[0:6];
----
[] [1.0, 2.0] [h, e, l, l, o]
query ???
select arrow_cast([1, 2, 3], 'LargeList(Int64)')[0:0],
arrow_cast([1.0, 2.0, 3.0], 'LargeList(Int64)')[0:2],
arrow_cast(['h', 'e', 'l', 'l', 'o'], 'LargeList(Utf8)')[0:6]
;
----
[] [1, 2] [h, e, l, l, o]
query I
select arrow_cast([1, 2, 3], 'LargeList(Int64)')[1];
----
1
# TODO: support multiple negative index
# multiple index with columns #3 (negative index)
# query II
# select make_array(1, 2, 3)[-3:-1], make_array(1.0, 2.0, 3.0)[-3:-1], make_array('h', 'e', 'l', 'l', 'o')[-2:0];
# ----
# TODO: support complex index
# multiple index with columns #4 (complex index)
# query III
# select make_array(1, 2, 3)[2 + 1 - 1:10], make_array(1.0, 2.0, 3.0)[2 | 2:10], make_array('h', 'e', 'l', 'l', 'o')[6 ^ 6:10];
# ----
# multiple index with columns #1 (positive index)
query ???
select column1[2:4], column2[1:4], column3[3:4] from arrays;
----
[[3, ]] [1.1, 2.2, 3.3] [r, e]
[[5, 6]] [, 5.5, 6.6] [, u]
[[7, 8]] [7.7, 8.8, 9.9] [l, o]
[[9, 10]] [10.1, , 12.2] [t]
[] [13.3, 14.4, 15.5] [e, t]
[[13, 14]] [] []
[[, 18]] [16.6, 17.7, 18.8] []
# multiple index with columns #2 (zero index)
query ???
select column1[0:5], column2[0:3], column3[0:9] from arrays;
----
[[, 2], [3, ]] [1.1, 2.2, 3.3] [L, o, r, e, m]
[[3, 4], [5, 6]] [, 5.5, 6.6] [i, p, , u, m]
[[5, 6], [7, 8]] [7.7, 8.8, 9.9] [d, , l, o, r]
[[7, ], [9, 10]] [10.1, , 12.2] [s, i, t]
[] [13.3, 14.4, 15.5] [a, m, e, t]
[[11, 12], [13, 14]] [] [,]
[[15, 16], [, 18]] [16.6, 17.7, 18.8] []
# TODO: support negative index
# multiple index with columns #3 (negative index)
# query ?RT
# select column1[-2:-4], column2[-3:-5], column3[-1:-4] from arrays;
# ----
# [, 2] 1.1 m
# TODO: support complex index
# multiple index with columns #4 (complex index)
# query ?RT
# select column1[9 - 7:2 + 2], column2[1 * 0:2 * 3], column3[1 + 1 - 0:5 % 3] from arrays;
# ----
# TODO: support first index as column
# multiple index with columns #5 (first index as column)
# query ?
# select make_array(1, 2, 3, 4, 5)[column2:4] from arrays_with_repeating_elements
# ----
# TODO: support last index as column
# multiple index with columns #6 (last index as column)
# query ?RT
# select make_array(1, 2, 3, 4, 5)[2:column3] from arrays_with_repeating_elements;
# ----
# TODO: support argument and indices as column
# multiple index with columns #7 (argument and indices as column)
# query ?RT
# select column1[column2:column3] from arrays_with_repeating_elements;
# ----
# array[i:j:k]
# multiple index with columns #1 (positive index)
query ???
select make_array(1, 2, 3)[1:2:2], make_array(1.0, 2.0, 3.0)[2:3:2], make_array('h', 'e', 'l', 'l', 'o')[2:4:2];
----
[1] [2.0] [e, l]
# multiple index with columns #2 (zero index)
query ???
select make_array(1, 2, 3)[0:0:2], make_array(1.0, 2.0, 3.0)[0:2:2], make_array('h', 'e', 'l', 'l', 'o')[0:6:2];
----
[] [1.0] [h, l, o]
#TODO: sqlparser does not support negative index
## multiple index with columns #3 (negative index)
#query ???
#select make_array(1, 2, 3)[-1:-2:-2], make_array(1.0, 2.0, 3.0)[-2:-3:-2], make_array('h', 'e', 'l', 'l', 'o')[-2:-4:-2];
#----
#[1] [2.0] [e, l]
# multiple index with columns #1 (positive index)
query ???
select column1[2:4:2], column2[1:4:2], column3[3:4:2] from arrays;
----
[[3, ]] [1.1, 3.3] [r]
[[5, 6]] [, 6.6] []
[[7, 8]] [7.7, 9.9] [l]
[[9, 10]] [10.1, 12.2] [t]
[] [13.3, 15.5] [e]
[[13, 14]] [] []
[[, 18]] [16.6, 18.8] []
# multiple index with columns #2 (zero index)
query ???
select column1[0:5:2], column2[0:3:2], column3[0:9:2] from arrays;
----
[[, 2]] [1.1, 3.3] [L, r, m]
[[3, 4]] [, 6.6] [i, , m]
[[5, 6]] [7.7, 9.9] [d, l, r]
[[7, ]] [10.1, 12.2] [s, t]
[] [13.3, 15.5] [a, e]
[[11, 12]] [] [,]
[[15, 16]] [16.6, 18.8] []
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "jmhain (via GitHub)" <gi...@apache.org>.
jmhain commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1606083522
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
https://github.com/sqlparser-rs/sqlparser-rs/pull/1278
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143340101
@alamb there is even a new breaking change at https://github.com/sqlparser-rs/sqlparser-rs/pull/1272:
```
error[E0277]: the trait bound `Arc<str>: From<Box<sqlparser::ast::Expr>>` is not satisfied
--> datafusion/sql/src/expr/mod.rs:578:74
|
578 | DataType::Timestamp(TimeUnit::Nanosecond, Some(time_zone.into())),
| ^^^^ the trait `From<Box<sqlparser::ast::Expr>>` is not implemented for `Arc<str>`, which is required by `Box<sqlparser::ast::Expr>: Into<_>`
|
= help: the following other types implement trait `From<T>`:
<Arc<str> as From<&str>>
<Arc<str> as From<std::string::String>>
= note: required for `Box<sqlparser::ast::Expr>` to implement `Into<Arc<str>>`
```
I'm worried about downstream projects to upgrade datafusion now 🤣
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1617979884
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -215,11 +216,28 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
_ => (expr, false),
}
}
+
let expr =
- self.sql_expr_to_logical_expr(*obj, schema, planner_context)?;
+ self.sql_expr_to_logical_expr(*expr, schema, planner_context)?;
+
+ let indexes = match *subscript {
+ Subscript::Index { index } => vec![index],
+ Subscript::Slice { lower_bound, upper_bound } => {
+ let mut indexes = vec![];
+ if let Some(lower_bound) = lower_bound {
+ indexes.push(lower_bound);
+ }
+ if let Some(upper_bound) = upper_bound {
+ indexes.push(upper_bound);
+ }
+ indexes
+ }
+ };
Review Comment:
Seems we need adapt `plan_indices` with this new `Subscript` struct and it may be wrapped into a `SQLExpr` struct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2148515761
@alamb Thanks for your review and follow-up! @jmhain Thanks for your help :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1626434201
##########
.github/workflows/rust.yml:
##########
@@ -334,6 +334,8 @@ jobs:
run: |
export PATH=$PATH:$HOME/d/protoc/bin
cargo test --lib --tests --bins --features avro,json,backtrace
+ # 8MB stack size otherwise the cli_integration test fails
Review Comment:
I disabled the test and filed https://github.com/apache/datafusion/issues/10793 to track
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1600616490
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -679,55 +661,6 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
)))
}
- fn parse_array_agg(
- &self,
- array_agg: ArrayAgg,
- input_schema: &DFSchema,
- planner_context: &mut PlannerContext,
- ) -> Result<Expr> {
- // Some dialects have special syntax for array_agg. DataFusion only supports it like a function.
Review Comment:
Sqlparser now handles this without a special case -- in https://github.com/sqlparser-rs/sqlparser-rs/pull/1247
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1600620737
##########
datafusion/sqllogictest/test_files/array.slt:
##########
@@ -689,7 +689,7 @@ select column1, column2, column3, column4, column5 from nested_arrays;
# values table
query IIIRT
-select a, b, c, d, e from values;
+select a, b, c, d, e from "values";
Review Comment:
it seems that `values` can no longer be parsed as a bare table name (I am not sure why, to be honest)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2111041494
I am going to give this PR a look to see if I can help unstick it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1600649538
##########
datafusion/sqllogictest/test_files/timestamps.slt:
##########
@@ -2795,3 +2795,10 @@ SELECT '2000-12-01 04:04:12' AT TIME ZONE 'America/New York';
# abbreviated timezone is not supported
statement error
SELECT '2023-03-12 02:00:00' AT TIME ZONE 'EDT';
+
+
+# Test current_time without parentheses
Review Comment:
While updating the code I realized I almost broke this, so I added a new test (also propose adding it to `main` here: https://github.com/apache/datafusion/pull/10509)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1599302595
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -943,11 +950,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
) => GetFieldAccess::NamedStructField {
name: ScalarValue::from(s),
},
- SQLExpr::JsonAccess {
- left,
- operator: JsonOperator::Colon,
- right,
- } => {
+ SQLExpr::JsonAccess { value, path } => {
Review Comment:
I pushed f29af9d2ae19c4209d39f90cd7c5d6331a1eb6bd to try it out.
The rest agg parts look much wider that I may hardly handle it by myself in a short time ..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2109169882
Seems that we should merge `AggregateExpressionWithFilter` and `ArrayAgg` handle logic into `sql_function_to_expr`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1617967562
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -215,11 +216,28 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
_ => (expr, false),
}
}
+
let expr =
- self.sql_expr_to_logical_expr(*obj, schema, planner_context)?;
+ self.sql_expr_to_logical_expr(*expr, schema, planner_context)?;
+
+ let indexes = match *subscript {
+ Subscript::Index { index } => vec![index],
+ Subscript::Slice { lower_bound, upper_bound } => {
+ let mut indexes = vec![];
+ if let Some(lower_bound) = lower_bound {
+ indexes.push(lower_bound);
+ }
+ if let Some(upper_bound) = upper_bound {
+ indexes.push(upper_bound);
+ }
+ indexes
+ }
+ };
Review Comment:
Now we pass the compilation. But it still fails `array.slt`:
```
External error: query failed: DataFusion error: Error during planning: Error during planning: Coercion from [Int64, Int64] to the signature ArraySignature(ArrayAndIndex) failed. and No function matches the given name and argument types 'array_element(Int64, Int64)'. You might need to add explicit type casts.
Candidate functions:
array_element(array, index)
[SQL] select make_array(1, 2, 3)[1:2], make_array(1.0, 2.0, 3.0)[2:3], make_array('h', 'e', 'l', 'l', 'o')[2:4];
at test_files/array.slt:859
Error: Execution("1 failures")
error: test failed, to rerun pass `-p datafusion-sqllogictest --test sqllogictests`
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1617977560
##########
datafusion/sql/src/expr/mod.rs:
##########
@@ -215,11 +216,28 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
_ => (expr, false),
}
}
+
let expr =
- self.sql_expr_to_logical_expr(*obj, schema, planner_context)?;
+ self.sql_expr_to_logical_expr(*expr, schema, planner_context)?;
+
+ let indexes = match *subscript {
+ Subscript::Index { index } => vec![index],
+ Subscript::Slice { lower_bound, upper_bound } => {
+ let mut indexes = vec![];
+ if let Some(lower_bound) = lower_bound {
+ indexes.push(lower_bound);
+ }
+ if let Some(upper_bound) = upper_bound {
+ indexes.push(upper_bound);
+ }
+ indexes
+ }
+ };
Review Comment:
For `make_array(1, 2, 3)[1:2]`, previous AST:
```
ArrayIndex {
obj: ...,
indexes: [
JsonAccess {
left: Value(Number("1", false)),
operator: Colon,
right: Value(Number("2", false)),
},
],
}
```
new AST:
```
Subscript {
expr: ...,
subscript: Slice {
lower_bound: Some(Value(Number("1", false))),
upper_bound: Some(Value(Number("2", false))),
},
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2141013364
@alamb Thank you and cool! I'll update this PR once sqlparser released a 0.47.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143357574
Window specific test failed. I don't have a windows env to debug ...
```
test cli_quick_test::case_4_set_batch_size ... FAILED
failures:
---- cli_quick_test::case_4_set_batch_size stdout ----
thread 'cli_quick_test::case_4_set_batch_size' panicked at /rustc/9b00956e56009bab2aa15d7bff10916599e3d6d6\library\core\src\ops\function.rs:250:5:
Unexpected stdout, failed var == "[{\"name\":\"datafusion.execution.batch_size\",\"value\":\"1\"}]\n"
├── var: ""
└── var as str:
command=`"D:\\a\\datafusion\\datafusion\\datafusion-cli\\target\\debug\\datafusion-cli.exe" "--command" "show datafusion.execution.batch_size" "--format" "json" "-q" "-b" "1"`
code=-1073741571
stdout=""
stderr=```
thread \'main\' has overflowed its stack
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1623198710
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
👍
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
FYI @milenkovicm -- sqlparser changed how the body was represented so now this changes as well to a generic Expr
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143383182
> @alamb there is even a new breaking change at [sqlparser-rs/sqlparser-rs#1272](https://github.com/sqlparser-rs/sqlparser-rs/pull/1272):
This one is straightforward -- the change was that the timezone can now be an arbitrary expression
So I think the change looks something like `timezone.to_string().into()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.46.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#issuecomment-2143345616
Since we're on the sqlparser-rs main branch now, I'd consider this patch as ready to merge.
But of course it's better to have a new release to sqlparser-rs and we move to that version. Hopefully it contains what it has now without new breaking changes :P
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] build(deps): upgrade sqlparser to 0.47.0 [datafusion]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #10392:
URL: https://github.com/apache/datafusion/pull/10392#discussion_r1624648204
##########
datafusion/expr/src/logical_plan/ddl.rs:
##########
@@ -341,29 +341,8 @@ pub struct CreateFunctionBody {
pub language: Option<Ident>,
/// IMMUTABLE | STABLE | VOLATILE
pub behavior: Option<Volatility>,
- /// AS 'definition'
- pub as_: Option<DefinitionStatement>,
- /// RETURN expression
- pub return_: Option<Expr>,
-}
-
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
-pub enum DefinitionStatement {
- SingleQuotedDef(String),
- DoubleDollarDef(String),
-}
-
-impl From<sqlparser::ast::FunctionDefinition> for DefinitionStatement {
- fn from(value: sqlparser::ast::FunctionDefinition) -> Self {
- match value {
- sqlparser::ast::FunctionDefinition::SingleQuotedDef(s) => {
- Self::SingleQuotedDef(s)
- }
- sqlparser::ast::FunctionDefinition::DoubleDollarDef(s) => {
- Self::DoubleDollarDef(s)
- }
- }
- }
+ /// RETURN or AS function body
+ pub function_body: Option<Expr>,
Review Comment:
IIRC we should previously support `$$` function body. If it fails to parse now, it's a regression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org