You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "jnlt3 (via GitHub)" <gi...@apache.org> on 2025/12/08 12:31:55 UTC

[PR] PostgreSQL Tokenization: Fix unexpected characters after question mark being silently ignored [datafusion-sqlparser-rs]

jnlt3 opened a new pull request, #2129:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2129

   ## Description of Issue
   Upon encountering '?', the tokenizer first consumes the token and peeks to match any the following: '|' , '&', '-', '#'. If none of the symbols are present it will call `consume_and_return(chars, Token::Question)` which consumes an additional character but only returns a `Token::Question`.  This is also reflected in `tokenize_with_location` where the relevant `Token::Question` will have a span of 2 characters. 
   
   ## Reproducing the Issue
   Both tests will fail on the current main branch.
   ```rs
     use sqlparser::{
         dialect::PostgreSqlDialect,
         tokenizer::{Token, TokenWithSpan, Tokenizer},
     };
   
     #[test]
     pub fn example_1() {
         let query_lhs = "x?";
         let query_rhs = "x?a";
         let tokens_lhs = Tokenizer::new(&PostgreSqlDialect {}, query_lhs)
             .tokenize()
             .unwrap();
         let tokens_rhs = Tokenizer::new(&PostgreSqlDialect {}, query_rhs)
             .tokenize()
             .unwrap();
         assert_ne!(tokens_lhs, tokens_rhs);
     }
   
     #[test]
     pub fn example_2() {
         let tokens = Tokenizer::new(&PostgreSqlDialect {}, "x?a")
             .tokenize_with_location()
             .unwrap();
         for token in tokens {
             if let TokenWithSpan {
                 token: Token::Question,
                 span,
             } = token
             {
                 assert_eq!(span.start.column + 1, span.end.column)
             }
         }
     }
   ```
   
   ## The Proposed Fix
   The PR replaces the call to `self.consume_and_return(chars, Token::Question)` with `Ok(Some(Token::Question))` no longer consuming the additional token. 
   
   ## Additional considerations
   As far as I am aware, `Token::Question` is not a valid PostgreSQL token and the best course of action might be to explicitly not support it. 


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

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


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


Re: [PR] PostgreSQL Tokenization: Fix unexpected characters after question mark being silently ignored [datafusion-sqlparser-rs]

Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio commented on code in PR #2129:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2129#discussion_r2629509575


##########
tests/sqlparser_postgres.rs:
##########
@@ -7492,3 +7492,20 @@ fn parse_create_operator_class() {
         )
         .is_err());
 }
+
+#[test]
+fn tokenize_question_mark() {
+    let sql = "SELECT x ? y";
+    pg().tokenizes_to(
+        sql,
+        vec![
+            Token::make_keyword("SELECT"),
+            Token::Whitespace(Whitespace::Space),
+            Token::make_word("x", None),
+            Token::Whitespace(Whitespace::Space),
+            Token::Question,
+            Token::Whitespace(Whitespace::Space),
+            Token::make_word("y", None),
+        ],
+    )
+}

Review Comment:
   I thnk the test can live in tokenizer.rs instead since its covering tokenizer specific behavior



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

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


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


Re: [PR] PostgreSQL Tokenization: Fix unexpected characters after question mark being silently ignored [datafusion-sqlparser-rs]

Posted by "jnlt3 (via GitHub)" <gi...@apache.org>.
jnlt3 commented on code in PR #2129:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2129#discussion_r2629834278


##########
tests/sqlparser_postgres.rs:
##########
@@ -7492,3 +7492,20 @@ fn parse_create_operator_class() {
         )
         .is_err());
 }
+
+#[test]
+fn tokenize_question_mark() {
+    let sql = "SELECT x ? y";
+    pg().tokenizes_to(
+        sql,
+        vec![
+            Token::make_keyword("SELECT"),
+            Token::Whitespace(Whitespace::Space),
+            Token::make_word("x", None),
+            Token::Whitespace(Whitespace::Space),
+            Token::Question,
+            Token::Whitespace(Whitespace::Space),
+            Token::make_word("y", None),
+        ],
+    )
+}

Review Comment:
   I have moved the test to `tokenizer.rs` and rebased on current main branch.
   



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

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


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


Re: [PR] PostgreSQL Tokenization: Fix unexpected characters after question mark being silently ignored [datafusion-sqlparser-rs]

Posted by "jnlt3 (via GitHub)" <gi...@apache.org>.
jnlt3 commented on PR #2129:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2129#issuecomment-3650873810

   > @jnlt3 could we add a test case demonstrating the change in behavior?
   
   I have added the test case. 
   
   I also stand corrected about one of my previous comments, `Token::Question` is in fact a valid PostgreSQL token. 


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

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


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


Re: [PR] PostgreSQL Tokenization: Fix unexpected characters after question mark being silently ignored [datafusion-sqlparser-rs]

Posted by "iffyio (via GitHub)" <gi...@apache.org>.
iffyio merged PR #2129:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2129


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

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


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