iffyio commented on code in PR #2192:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2192#discussion_r2769994204


##########
src/parser/mod.rs:
##########
@@ -1549,6 +1549,43 @@ impl<'a> Parser<'a> {
             Keyword::LAMBDA if self.dialect.supports_lambda_functions() => {
                 Ok(Some(self.parse_lambda_expr()?))
             }
+            Keyword::FILTER
+            | Keyword::TRANSFORM
+            | Keyword::REDUCE if self.dialect.supports_lambda_functions() => {

Review Comment:
   the condition looks incompatible with non-snowflake dialects that support 
lambda functions, since those I'm imagining treat filter/transform/reduce as 
identifiers instead?



##########
src/parser/mod.rs:
##########
@@ -1549,6 +1549,43 @@ impl<'a> Parser<'a> {
             Keyword::LAMBDA if self.dialect.supports_lambda_functions() => {
                 Ok(Some(self.parse_lambda_expr()?))
             }
+            Keyword::FILTER
+            | Keyword::TRANSFORM
+            | Keyword::REDUCE if self.dialect.supports_lambda_functions() => {
+                self.expect_token(&Token::LParen)?;

Review Comment:
   can we move the logic to a standalone function? since its a lot of code to 
inline



##########
src/parser/mod.rs:
##########
@@ -1549,6 +1549,43 @@ impl<'a> Parser<'a> {
             Keyword::LAMBDA if self.dialect.supports_lambda_functions() => {
                 Ok(Some(self.parse_lambda_expr()?))
             }
+            Keyword::FILTER
+            | Keyword::TRANSFORM
+            | Keyword::REDUCE if self.dialect.supports_lambda_functions() => {
+                self.expect_token(&Token::LParen)?;
+                let array = self.parse_expr()?;

Review Comment:
   would it make sense to merge this and the existing `parse_lambda_expr`? that 
we accept arbitrary expressions as the parameters of the lambda instead of the 
latter only accepting identifiers? Similarly with the `typed` bool since the 
impl already uses `maybe_parse(data_type)`, thinking we can always accept a 
datatype if present in the syntax



##########
src/ast/mod.rs:
##########
@@ -1445,6 +1445,26 @@ impl fmt::Display for LambdaFunction {
     }
 }
 
+/// A parameter to a lambda function, optionally with a data type.
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub struct LambdaFunctionParameter {
+    /// The name of the parameter
+    pub name: Ident,
+    /// The optional data type of the parameter
+    pub data_type: Option<DataType>,

Review Comment:
   Can we add a link to the docs containing the data_type syntax?



##########
tests/sqlparser_snowflake.rs:
##########
@@ -4559,3 +4559,11 @@ fn test_truncate_table_if_exists() {
     snowflake().verified_stmt("TRUNCATE TABLE my_table");
     snowflake().verified_stmt("TRUNCATE IF EXISTS my_table");
 }
+
+#[test]
+fn test_snowflake_lambda() {
+    snowflake().verified_expr("TRANSFORM([1, 2, 3], a -> a * 2)");
+    snowflake().verified_expr("TRANSFORM([1, 2, 3], a INT -> a * 2)");
+    snowflake().verified_expr("TRANSFORM([1, 2, 3], (x INT, y INT) -> (x + 
y))");
+    snowflake().verified_expr("REDUCE([1, 2, 3], 0, (acc, val) -> acc + val)");

Review Comment:
   can we add test cases for `FILTER` as well?



-- 
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]

Reply via email to