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


##########
src/parser/mod.rs:
##########
@@ -11384,6 +11406,23 @@ impl<'a> Parser<'a> {
         })
     }
 
+    /// Parses MSSQL's json-null-clause
+    fn parse_json_null_clause(&mut self) -> Option<JsonNullClause> {
+        if !dialect_of!(self is MsSqlDialect) {
+            return None;
+        }
+        if let Some(kw) = self.parse_one_of_keywords(&[Keyword::ABSENT, 
Keyword::NULL]) {
+            if self.parse_keywords(&[Keyword::ON, Keyword::NULL]) {
+                if kw == Keyword::ABSENT {
+                    return Some(JsonNullClause::AbsentOnNull);
+                } else if kw == Keyword::NULL {
+                    return Some(JsonNullClause::NullOnNull);
+                }
+            }
+        }

Review Comment:
   could we go back to the previous version of this logic? I think that was 
more readable since we avoided conditional state, and it read the same as the 
syntax. (e.g its no longer super clear what's happening if we match `ABSENT` 
but not the `ON NULL` part)
   ```rust
   if self.parse_keywords(absent, on, null) {
      Some()
   else if self.parse_keywords(null, on, null) {
      Some()
   } else {
       None
   }
   ```



##########
src/dialect/mod.rs:
##########
@@ -231,11 +231,32 @@ pub trait Dialect: Debug + Any {
         false
     }
 
-    /// Returns true if the dialect supports named arguments of the form FUN(a 
= '1', b = '2').
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a = '1', b = '2')`.
     fn supports_named_fn_args_with_eq_operator(&self) -> bool {
         false
     }
 
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a : '1', b : '2')`.
+    fn supports_named_fn_args_with_colon_operator(&self) -> bool {
+        false
+    }
+
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a := '1', b := '2')`.
+    fn supports_named_fn_args_with_assignment_operator(&self) -> bool {
+        false
+    }
+
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a => '1', b => '2')`.
+    fn supports_named_fn_args_with_rarrow_operator(&self) -> bool {
+        true //for compatible

Review Comment:
   ```suggestion
           true
   ```



##########
src/parser/mod.rs:
##########
@@ -11384,6 +11406,23 @@ impl<'a> Parser<'a> {
         })
     }
 
+    /// Parses MSSQL's json-null-clause
+    fn parse_json_null_clause(&mut self) -> Option<JsonNullClause> {
+        if !dialect_of!(self is MsSqlDialect) {
+            return None;
+        }

Review Comment:
   here we can use a dialect method as well?



##########
src/dialect/mod.rs:
##########
@@ -231,11 +231,32 @@ pub trait Dialect: Debug + Any {
         false
     }
 
-    /// Returns true if the dialect supports named arguments of the form FUN(a 
= '1', b = '2').
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a = '1', b = '2')`.
     fn supports_named_fn_args_with_eq_operator(&self) -> bool {
         false
     }
 
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a : '1', b : '2')`.
+    fn supports_named_fn_args_with_colon_operator(&self) -> bool {
+        false
+    }
+
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a := '1', b := '2')`.
+    fn supports_named_fn_args_with_assignment_operator(&self) -> bool {
+        false
+    }
+
+    /// Returns true if the dialect supports named arguments of the form 
`FUN(a => '1', b => '2')`.
+    fn supports_named_fn_args_with_rarrow_operator(&self) -> bool {
+        true //for compatible
+    }
+
+    /// Returns true if dialect supports argument name as arbitrary expression 
(`FUN(<arbitrary-expr>:<arbitrary-expr>,...)`),and will use the 
`FunctionArg::ExprNamed` variant,

Review Comment:
   ```suggestion
       /// Returns true if dialect supports argument name as arbitrary 
expression.
       /// e.g. `FUN(LOWER('a'):'1',  b:'2')`
       /// Such function arguments are represented in the AST by the 
`FunctionArg::ExprNamed` variant,
   ```



-- 
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-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to