LucaCappelletti94 commented on code in PR #2458:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2458#discussion_r3889759338
##########
tests/sqlparser_derive_dialect.rs:
##########
@@ -121,3 +125,65 @@ fn test_identifier_quote_style_overrides() {
None
);
}
+
+#[test]
+fn test_lambda_keyword_syntax_on_postgres_derivative() {
+ // 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);
+}
Review Comment:
I believe we should add a test for the Spark/Snowflake case (they only
accept `->`):
```suggestion
}
#[test]
fn custom_dialect_lambda_arrow_syntax_without_keyword() {
// Arrow lambdas stay on while the `LAMBDA` keyword spelling is off,
// as in engines like Spark and Snowflake.
#[derive(Debug)]
struct MyDialect {}
impl Dialect for MyDialect {
fn is_identifier_start(&self, ch: char) -> bool {
is_identifier_start(ch)
}
fn is_identifier_part(&self, ch: char) -> bool {
is_identifier_part(ch)
}
fn supports_lambda_functions(&self) -> bool {
true
}
fn supports_lambda_keyword_syntax(&self) -> bool {
false
}
}
let dialect = MyDialect {};
let sql = "SELECT transform(xs, x -> x + 1)";
assert_eq!(
sql,
&format!("{}", Parser::parse_sql(&dialect, sql).unwrap()[0])
);
assert!(Parser::parse_sql(&dialect, "SELECT transform(xs, lambda x : x +
1)").is_err());
}
```
##########
src/dialect/mod.rs:
##########
@@ -535,10 +535,30 @@ 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 transform(array(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.
+ fn supports_lambda_keyword_syntax(&self) -> bool {
+ self.supports_lambda_functions()
+ }
Review Comment:
Arrow-only is already expressible by overriding
`supports_lambda_keyword_syntax` to `false`, so I believe there is no need.
##########
src/dialect/mod.rs:
##########
@@ -535,10 +535,30 @@ 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 transform(array(1, 2, 3), LAMBDA x : x + 1); -- returns [2,3,4]
Review Comment:
I believe `transform(array(...), LAMBDA ...)` is the wrong spelling, if you
meant the DuckDB syntax it would be more like:
```suggestion
/// SELECT list_transform([1, 2, 3], lambda x : x + 1); -- returns [2,
3, 4]
```
##########
tests/sqlparser_derive_dialect.rs:
##########
@@ -121,3 +123,45 @@ fn test_identifier_quote_style_overrides() {
None
);
}
+
+#[test]
+fn test_lambda_keyword_syntax_on_postgres_derivative() {
Review Comment:
Both seem worth keeping.
##########
src/dialect/mod.rs:
##########
@@ -535,10 +535,30 @@ 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 transform(array(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.
+ fn supports_lambda_keyword_syntax(&self) -> bool {
Review Comment:
```suggestion
///
/// See <https://duckdb.org/docs/stable/sql/functions/lambda>
fn supports_lambda_keyword_syntax(&self) -> bool {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
