7phs commented on code in PR #1576:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/1576#discussion_r1867993041


##########
src/dialect/redshift.rs:
##########
@@ -41,10 +41,19 @@ impl Dialect for RedshiftSqlDialect {
     /// treating them as json path. If there is identifier then we assume
     /// there is no json path.
     fn is_proper_identifier_inside_quotes(&self, mut chars: 
Peekable<Chars<'_>>) -> bool {
+        // PartiQL (used as json path query language in Redshift) uses square 
bracket as a start character and a quote is a beginning of quoted identifier
+        if let Some(quote_start) = chars.peek() {
+            if *quote_start == '"' {
+                return true;
+            }
+        };
         chars.next();
         let mut not_white_chars = chars.skip_while(|ch| 
ch.is_whitespace()).peekable();
         if let Some(&ch) = not_white_chars.peek() {
-            return self.is_identifier_start(ch);
+            // PartiQL uses single quote as starting identification inside a 
quote
+            // It is a normal identifier if it has no single quote at the 
beginning.
+            // Additionally square bracket can contain quoted identifier.
+            return ch == '"' || ch != '\'' && self.is_identifier_start(ch);

Review Comment:
   This method is called by 
[tokeniser](https://github.com/apache/datafusion-sqlparser-rs/blob/main/src/tokenizer.rs#L1078):
   ```
                   quote_start
                       if self.dialect.is_delimited_identifier_start(ch)
                           && self
                               .dialect
                               
.is_proper_identifier_inside_quotes(chars.peekable.clone()) =>
   ```
   It applies to both delimiters of identifiers (`"` and `[`) to determine 
whether the current character sequence marks the beginning of a quoted token. 
This code is executed regardless of the specific characters—be it `[0]` or 
`"a"`.
   
   However, only `[` is considered the start of a quoted identifier in json 
path case. This PR clarifies this behavior with a more explicit condition at 
the start of the method.
   
   The highlighted line can only be triggered for `[0]` and never for `"a"`. 
However, there is a possibility that a double(by number of different type of 
symbols)-quoted identifier like `["a"]` could be parsed. An additional check 
for `"` addresses this scenario. Note that JSON path expressions use single 
quotes `['a']` and never double quotes.
   
   I added unit-tests to cover all these cases.



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