-
Notifications
You must be signed in to change notification settings - Fork 767
Allow dialects that use -> as an operator to support LAMBDA syntax
#2458
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
base: main
Are you sure you want to change the base?
Changes from all commits
6753b2f
4c58d2e
1cebea9
c91244d
1d89c66
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 |
|---|---|---|
|
|
@@ -535,10 +535,32 @@ pub trait Dialect: Debug + Any { | |
| /// ```sql | ||
| /// SELECT transform(array(1, 2, 3), x -> x + 1); -- returns [2,3,4] | ||
| /// ``` | ||
| /// | ||
| /// This enables both the `->` spelling above and the `LAMBDA` keyword | ||
| /// spelling gated by [`Self::supports_lambda_keyword_syntax`]. A dialect | ||
| /// that uses `->` as a binary operator should override only the latter. | ||
| fn supports_lambda_functions(&self) -> bool { | ||
| false | ||
| } | ||
|
|
||
| /// Returns true if the dialect supports the `LAMBDA` keyword spelling of | ||
| /// lambda functions, for example: | ||
| /// | ||
| /// ```sql | ||
| /// SELECT list_transform([1, 2, 3], lambda x : x + 1); -- returns [2, 3, 4] | ||
| /// ``` | ||
| /// | ||
| /// This spelling does not claim the `->` token, so it can be enabled by | ||
| /// dialects that already give `->` a different meaning, such as PostgreSQL | ||
| /// and its derivatives, where `->` is JSON member access. Defaults to | ||
| /// [`Self::supports_lambda_functions`], so dialects supporting the `->` | ||
| /// spelling accept the `LAMBDA` spelling too unless they say otherwise. | ||
| /// | ||
| /// See <https://duckdb.org/docs/stable/sql/functions/lambda> | ||
| fn supports_lambda_keyword_syntax(&self) -> bool { | ||
| self.supports_lambda_functions() | ||
| } | ||
|
Comment on lines
+560
to
+562
Author
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. One could argue for adding
Contributor
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. Arrow-only is already expressible by overriding |
||
|
|
||
| /// Returns true if the dialect supports multiple variable assignment | ||
| /// using parentheses in a `SET` variable declaration. | ||
| /// | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -17,9 +17,13 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| //! Tests for the `derive_dialect!` macro. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use sqlparser::ast::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| BinaryOperator, Expr, FunctionArg, FunctionArgExpr, FunctionArguments, LambdaSyntax, Statement, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use sqlparser::derive_dialect; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use sqlparser::dialect::{Dialect, GenericDialect, MySqlDialect, PostgreSqlDialect}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use sqlparser::parser::Parser; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use sqlparser::test_utils::{expr_from_projection, only}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn test_method_overrides() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -121,3 +125,65 @@ fn test_identifier_quote_style_overrides() { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[test] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn test_lambda_keyword_syntax_on_postgres_derivative() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+129
to
+130
Author
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'm split between this (the real regression test I want) and another test using a
Contributor
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. Both seem worth keeping. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // A PostgreSQL derivative can opt into the `LAMBDA` keyword spelling of | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // lambda functions without giving up `->` as JSON member access. The two | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // meet in a single expression below: a lambda whose body is a JSON access. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| derive_dialect!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LambdaPostgreSqlDialect, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PostgreSqlDialect, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| overrides = { supports_lambda_keyword_syntax = true } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let dialect = LambdaPostgreSqlDialect::new(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Only the keyword spelling is enabled; the arrow spelling stays off. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert!(dialect.supports_lambda_keyword_syntax()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert!(!dialect.supports_lambda_functions()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let sql = "SELECT transform(xs, lambda x : (x -> 'a')::INT + 1)"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let ast = Parser::parse_sql(&dialect, sql).unwrap(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(sql, ast[0].to_string()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Round-tripping alone would not distinguish a JSON access from a nested | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // lambda, since both print as `x -> 'a'`, so check the parsed shape. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Statement::Query(query) = &ast[0] else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("unexpected statement {}", ast[0]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Expr::Function(func) = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expr_from_projection(only(&query.body.as_select().unwrap().projection)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected a function call"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let FunctionArguments::List(args) = &func.args else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected an argument list"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let [_, FunctionArg::Unnamed(FunctionArgExpr::Expr(Expr::Lambda(lambda)))] = &args.args[..] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected the second argument to be a lambda"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // The lambda came from the `LAMBDA` keyword, not from `->`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(LambdaSyntax::LambdaKeyword, lambda.syntax); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // And the `->` in its body is still JSON member access. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Expr::BinaryOp { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| left, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| op: BinaryOperator::Plus, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } = lambda.body.as_ref() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected the lambda body to be an addition"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Expr::Cast { expr, .. } = left.as_ref() else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected the left operand to be a cast"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Expr::Nested(json_access) = expr.as_ref() else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected the cast operand to be parenthesized"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let Expr::BinaryOp { op, .. } = json_access.as_ref() else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| panic!("expected `->` to stay a binary operator"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert_eq!(&BinaryOperator::Arrow, op); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
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 believe we should add a test for the Spark/Snowflake case (they only accept
Suggested change
Author
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. Thanks, added but in |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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.
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, applied!