LucaCappelletti94 commented on code in PR #2917:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2917#discussion_r4543567483
##########
tests/sqlparser_common.rs:
##########
@@ -21938,45 +21938,31 @@ fn parse_compound_field_access_numeric_display() {
}
#[test]
-fn parse_placeholder_disallows_quoted_ident() {
+fn parse_quoted_placeholders_roundtrip() {
let dialects = TestedDialects::new(vec![
Box::new(AnsiDialect {}),
Box::new(GenericDialect {}),
Box::new(SnowflakeDialect {}),
Box::new(SQLiteDialect {}),
]);
- // Valid placeholders roundtrip
- dialects.verified_stmt("SELECT :x");
-
- // Quoted identifiers are not valid placeholders
- let err = dialects.parse_sql_statements("SELECT :`a`").unwrap_err();
- assert_eq!(
- ParserError::ParserError("Expected: placeholder, found:
`a`".to_string()),
- err
- );
- let err = dialects.parse_sql_statements("SELECT :\"a\"").unwrap_err();
- assert_eq!(
- ParserError::ParserError("Expected: placeholder, found:
\"a\"".to_string()),
- err
- );
- let err = dialects.parse_sql_statements("SELECT:` a` a").unwrap_err();
- assert_eq!(
- ParserError::ParserError("Expected: placeholder, found: `
a`".to_string()),
- err
- );
-
+ for sql in [
+ "SELECT :x",
+ "SELECT :`a`",
+ r#"SELECT :"a""#,
+ "SELECT :` a` AS a",
+ r#"SELECT :"a b c", :"a""b""#,
+ ] {
+ dialects.verified_stmt(sql);
+ }
Review Comment:
You should assert the parsed value as well. `verified_stmt` only compares
the printed SQL, so storing the bare name and re-quoting it in `Display` would
keep every case green while callers such as SQLPage silently receive a
different `Placeholder` string while we wouldn't notice the regression.
```suggestion
}
let select = dialects.verified_only_select(r#"SELECT :"a b c""#);
assert_eq!(
expr_from_projection(only(&select.projection)),
&Expr::value(Value::Placeholder(r#":"a b c""#.to_string()))
);
```
--
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]
