-
Notifications
You must be signed in to change notification settings - Fork 723
Extend lambda support for ClickHouse and DuckDB dialects #1686
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
Changes from 3 commits
94e71ff
848dc65
adf24f9
353f7d7
dec3856
ec66e02
58b423e
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 |
|---|---|---|
|
|
@@ -340,12 +340,21 @@ pub trait Dialect: Debug + Any { | |
| /// Returns true if the dialect supports lambda functions, for example: | ||
| /// | ||
| /// ```sql | ||
| /// SELECT transform(array(1, 2, 3), x -> x + 1); -- returns [2,3,4] | ||
| /// SELECT transform(array(1, 2, 3), (x) -> x + 1); -- returns [2,3,4] | ||
| /// ``` | ||
| fn supports_lambda_functions(&self) -> bool { | ||
| false | ||
| } | ||
|
|
||
| /// Returns true if the dialect supports lambda functions without parentheses for a single argument, for example: | ||
| /// | ||
| /// ```sql | ||
| /// SELECT transform(array(1, 2, 3), x -> x + 1); -- returns [2,3,4] | ||
| /// ``` | ||
| fn supports_parensless_lambda_functions(&self) -> bool { | ||
|
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. If I understand correctly the idea with introducing this method is to avoid the Generic dialect's syntax conflict due to its pg json syntax support? If so I'm thinking it could more sense to turn off
Contributor
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. You got it right. I think your point makes sense and I'm fine with turning off cc @samuelcolvin in case you have interest in this too
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 don't have much context here, personally I think we'll want to switch off lambdas.
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. hmm, given expressions My thinking was indeed to potentially turn off generic dialect and remove
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. Agreed. The lambda syntax looks identical to Pg JSON lookup syntax with a completely different meaning.
Contributor
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. Agreed. |
||
| false | ||
| } | ||
|
|
||
| /// Returns true if the dialect supports method calls, for example: | ||
| /// | ||
| /// ```sql | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13285,3 +13285,98 @@ fn test_trailing_commas_in_from() { | |
| "SELECT 1, 2 FROM (SELECT * FROM t1), (SELECT * FROM t2)", | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_lambdas() { | ||
|
Contributor
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. Moved from databricks tests |
||
| let dialects = all_dialects_where(|d| d.supports_lambda_functions()); | ||
|
|
||
| #[rustfmt::skip] | ||
| let sql = concat!( | ||
| "SELECT array_sort(array('Hello', 'World'), ", | ||
| "(p1, p2) -> CASE WHEN p1 = p2 THEN 0 ", | ||
| "WHEN reverse(p1) < reverse(p2) THEN -1 ", | ||
| "ELSE 1 END)", | ||
| ); | ||
| pretty_assertions::assert_eq!( | ||
| SelectItem::UnnamedExpr(call( | ||
| "array_sort", | ||
| [ | ||
| call( | ||
| "array", | ||
| [ | ||
| Expr::Value(Value::SingleQuotedString("Hello".to_owned())), | ||
| Expr::Value(Value::SingleQuotedString("World".to_owned())) | ||
| ] | ||
| ), | ||
| Expr::Lambda(LambdaFunction { | ||
| params: OneOrManyWithParens::Many(vec![Ident::new("p1"), Ident::new("p2")]), | ||
| body: Box::new(Expr::Case { | ||
| operand: None, | ||
| conditions: vec![ | ||
| Expr::BinaryOp { | ||
| left: Box::new(Expr::Identifier(Ident::new("p1"))), | ||
| op: BinaryOperator::Eq, | ||
| right: Box::new(Expr::Identifier(Ident::new("p2"))) | ||
| }, | ||
| Expr::BinaryOp { | ||
| left: Box::new(call( | ||
| "reverse", | ||
| [Expr::Identifier(Ident::new("p1"))] | ||
| )), | ||
| op: BinaryOperator::Lt, | ||
| right: Box::new(call( | ||
| "reverse", | ||
| [Expr::Identifier(Ident::new("p2"))] | ||
| )) | ||
| } | ||
| ], | ||
| results: vec![ | ||
| Expr::Value(number("0")), | ||
| Expr::UnaryOp { | ||
| op: UnaryOperator::Minus, | ||
| expr: Box::new(Expr::Value(number("1"))) | ||
| } | ||
| ], | ||
| else_result: Some(Box::new(Expr::Value(number("1")))) | ||
| }) | ||
| }) | ||
| ] | ||
| )), | ||
| dialects.verified_only_select(sql).projection[0] | ||
| ); | ||
|
|
||
| dialects.verified_expr( | ||
| "map_zip_with(map(1, 'a', 2, 'b'), map(1, 'x', 2, 'y'), (k, v1, v2) -> concat(v1, v2))", | ||
| ); | ||
| dialects.verified_expr("transform(array(1, 2, 3), (x) -> x + 1)"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_parensless_lambdas() { | ||
| let dialects = all_dialects_where(|d| d.supports_parensless_lambda_functions()); | ||
|
|
||
| pretty_assertions::assert_eq!( | ||
| call( | ||
| "transform", | ||
| [ | ||
| call( | ||
| "array", | ||
| [ | ||
| Expr::Value(number("1")), | ||
| Expr::Value(number("2")), | ||
| Expr::Value(number("3")), | ||
| ] | ||
| ), | ||
| Expr::Lambda(LambdaFunction { | ||
| params: OneOrManyWithParens::One(Ident::new("x")), | ||
| body: Box::new(Expr::BinaryOp { | ||
| left: Box::new(Expr::Identifier(Ident::new("x"))), | ||
| op: BinaryOperator::Plus, | ||
| right: Box::new(Expr::Value(number("1"))) | ||
| }) | ||
| }) | ||
| ] | ||
| ), | ||
| dialects.verified_expr("transform(array(1, 2, 3), x -> x + 1)") | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,69 +83,6 @@ fn test_databricks_exists() { | |
| ); | ||
| } | ||
|
|
||
| #[test] | ||
|
Contributor
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. Moved into common tests |
||
| fn test_databricks_lambdas() { | ||
| #[rustfmt::skip] | ||
| let sql = concat!( | ||
| "SELECT array_sort(array('Hello', 'World'), ", | ||
| "(p1, p2) -> CASE WHEN p1 = p2 THEN 0 ", | ||
| "WHEN reverse(p1) < reverse(p2) THEN -1 ", | ||
| "ELSE 1 END)", | ||
| ); | ||
| pretty_assertions::assert_eq!( | ||
| SelectItem::UnnamedExpr(call( | ||
| "array_sort", | ||
| [ | ||
| call( | ||
| "array", | ||
| [ | ||
| Expr::Value(Value::SingleQuotedString("Hello".to_owned())), | ||
| Expr::Value(Value::SingleQuotedString("World".to_owned())) | ||
| ] | ||
| ), | ||
| Expr::Lambda(LambdaFunction { | ||
| params: OneOrManyWithParens::Many(vec![Ident::new("p1"), Ident::new("p2")]), | ||
| body: Box::new(Expr::Case { | ||
| operand: None, | ||
| conditions: vec![ | ||
| Expr::BinaryOp { | ||
| left: Box::new(Expr::Identifier(Ident::new("p1"))), | ||
| op: BinaryOperator::Eq, | ||
| right: Box::new(Expr::Identifier(Ident::new("p2"))) | ||
| }, | ||
| Expr::BinaryOp { | ||
| left: Box::new(call( | ||
| "reverse", | ||
| [Expr::Identifier(Ident::new("p1"))] | ||
| )), | ||
| op: BinaryOperator::Lt, | ||
| right: Box::new(call( | ||
| "reverse", | ||
| [Expr::Identifier(Ident::new("p2"))] | ||
| )) | ||
| } | ||
| ], | ||
| results: vec![ | ||
| Expr::Value(number("0")), | ||
| Expr::UnaryOp { | ||
| op: UnaryOperator::Minus, | ||
| expr: Box::new(Expr::Value(number("1"))) | ||
| } | ||
| ], | ||
| else_result: Some(Box::new(Expr::Value(number("1")))) | ||
| }) | ||
| }) | ||
| ] | ||
| )), | ||
| databricks().verified_only_select(sql).projection[0] | ||
| ); | ||
|
|
||
| databricks().verified_expr( | ||
| "map_zip_with(map(1, 'a', 2, 'b'), map(1, 'x', 2, 'y'), (k, v1, v2) -> concat(v1, v2))", | ||
| ); | ||
| databricks().verified_expr("transform(array(1, 2, 3), x -> x + 1)"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_values_clause() { | ||
| let values = Values { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.