Repository: apache/datafusion-sqlparser-rs (rust)

Issue:
Nested comments were broken by the sqlparser v0.54 release
Nested `/*` comments used to be parsed as such by sqlparser, but have been broken by the 0.54 release.

The following works in postgresql and mssql:

```
select /* /* comment */ */ 1;
```

and it used to be parsed correctly by sqlparser, but now is not.

Initially reported in https://github.com/sqlpage/SQLPage/issues/834

Pull request:
re-add support for nested comments in mssql
fix #1753

see https://learn.microsoft.com/en-us/sql/t-sql/language-elements/slash-star-comment-transact-sql?view=sql-server-ver16

Patch:
diff --git a/src/ast/mod.rs b/src/ast/mod.rs
index 72be3ff..554ec19 100644
--- a/src/ast/mod.rs
+++ b/src/ast/mod.rs
@@ -2956,10 +2956,8 @@ pub enum Statement {
     /// ```sql
     /// SET NAMES 'charset_name' [COLLATE 'collation_name']
     /// ```
-    ///
-    /// Note: this is a MySQL-specific statement.
     SetNames {
-        charset_name: String,
+        charset_name: Ident,
         collation_name: Option<String>,
     },
     /// ```sql
@@ -4684,8 +4682,7 @@ impl fmt::Display for Statement {
                 charset_name,
                 collation_name,
             } => {
-                f.write_str("SET NAMES ")?;
-                f.write_str(charset_name)?;
+                write!(f, "SET NAMES {}", charset_name)?;
 
                 if let Some(collation) = collation_name {
                     f.write_str(" COLLATE ")?;
diff --git a/src/dialect/generic.rs b/src/dialect/generic.rs
index 041d44b..c13d5aa 100644
--- a/src/dialect/generic.rs
+++ b/src/dialect/generic.rs
@@ -155,4 +155,8 @@ impl Dialect for GenericDialect {
     fn supports_match_against(&self) -> bool {
         true
     }
+
+    fn supports_set_names(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index 1c32bc5..aeb097c 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -201,6 +201,33 @@ pub trait Dialect: Debug + Any {
         false
     }
 
+    /// Determine whether the dialect strips the backslash when escaping LIKE wildcards (%, _).
+    ///
+    /// [MySQL] has a special case when escaping single quoted strings which leaves these unescaped
+    /// so they can be used in LIKE patterns without double-escaping (as is necessary in other
+    /// escaping dialects, such as [Snowflake]). Generally, special characters have escaping rules
+    /// causing them to be replaced with a different byte sequences (e.g. `'\0'` becoming the zero
+    /// byte), and the default if an escaped character does not have a specific escaping rule is to
+    /// strip the backslash (e.g. there is no rule for `h`, so `'\h' = 'h'`). MySQL's special case
+    /// for ignoring LIKE wildcard escapes is to *not* strip the backslash, so that `'\%' = '\\%'`.
+    /// This applies to all string literals though, not just those used in LIKE patterns.
+    ///
+    /// ```text
+    /// mysql> select '\_', hex('\\'), hex('_'), hex('\_');
+    /// +----+-----------+----------+-----------+
+    /// | \_ | hex('\\') | hex('_') | hex('\_') |
+    /// +----+-----------+----------+-----------+
+    /// | \_ | 5C        | 5F       | 5C5F      |
+    /// +----+-----------+----------+-----------+
+    /// 1 row in set (0.00 sec)
+    /// ```
+    ///
+    /// [MySQL]: https://dev.mysql.com/doc/refman/8.4/en/string-literals.html
+    /// [Snowflake]: https://docs.snowflake.com/en/sql-reference/functions/like#usage-notes
+    fn ignores_wildcard_escapes(&self) -> bool {
+        false
+    }
+
     /// Determine if the dialect supports string literals with `U&` prefix.
     /// This is used to specify Unicode code points in string literals.
     /// For example, in PostgreSQL, the following is a valid string literal:
@@ -953,6 +980,16 @@ pub trait Dialect: Debug + Any {
     fn supports_order_by_all(&self) -> bool {
         false
     }
+
+    /// Returns true if the dialect supports `SET NAMES <charset_name> [COLLATE <collation_name>]`.
+    ///
+    /// - [MySQL](https://dev.mysql.com/doc/refman/8.4/en/set-names.html)
+    /// - [Postgres](https://www.postgresql.org/docs/17/sql-set.html)
+    ///
+    /// Note: Postgres doesn't support the `COLLATE` clause, but we permissively parse it anyway.
+    fn supports_set_names(&self) -> bool {
+        false
+    }
 }
 
 /// This represents the operators for which precedence must be defined
diff --git a/src/dialect/mssql.rs b/src/dialect/mssql.rs
index 980f5ec..aeed1eb 100644
--- a/src/dialect/mssql.rs
+++ b/src/dialect/mssql.rs
@@ -95,4 +95,9 @@ impl Dialect for MsSqlDialect {
     fn supports_timestamp_versioning(&self) -> bool {
         true
     }
+
+    /// See <https://learn.microsoft.com/en-us/sql/t-sql/language-elements/slash-star-comment-transact-sql?view=sql-server-ver16>
+    fn supports_nested_comments(&self) -> bool {
+        true
+    }
 }
diff --git a/src/dialect/mysql.rs b/src/dialect/mysql.rs
index 8a0da87..0bdfc9b 100644
--- a/src/dialect/mysql.rs
+++ b/src/dialect/mysql.rs
@@ -62,6 +62,10 @@ impl Dialect for MySqlDialect {
         true
     }
 
+    fn ignores_wildcard_escapes(&self) -> bool {
+        true
+    }
+
     fn supports_numeric_prefix(&self) -> bool {
         true
     }
@@ -133,6 +137,10 @@ impl Dialect for MySqlDialect {
     fn supports_match_against(&self) -> bool {
         true
     }
+
+    fn supports_set_names(&self) -> bool {
+        true
+    }
 }
 
 /// `LOCK TABLES`
diff --git a/src/dialect/postgresql.rs b/src/dialect/postgresql.rs
index 57ed0b6..9b08b8f 100644
--- a/src/dialect/postgresql.rs
+++ b/src/dialect/postgresql.rs
@@ -254,4 +254,8 @@ impl Dialect for PostgreSqlDialect {
     fn supports_geometric_types(&self) -> bool {
         true
     }
+
+    fn supports_set_names(&self) -> bool {
+        true
+    }
 }
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index f234fcc..b11e577 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -10962,14 +10962,14 @@ impl<'a> Parser<'a> {
             OneOrManyWithParens::One(self.parse_object_name(false)?)
         };
 
-        if matches!(&variables, OneOrManyWithParens::One(variable) if variable.to_string().eq_ignore_ascii_case("NAMES")
-            && dialect_of!(self is MySqlDialect | GenericDialect))
-        {
+        let names = matches!(&variables, OneOrManyWithParens::One(variable) if variable.to_string().eq_ignore_ascii_case("NAMES"));
+
+        if names && self.dialect.supports_set_names() {
             if self.parse_keyword(Keyword::DEFAULT) {
                 return Ok(Statement::SetNamesDefault {});
             }
 
-            let charset_name = self.parse_literal_string()?;
+            let charset_name = self.parse_identifier()?;
             let collation_name = if self.parse_one_of_keywords(&[Keyword::COLLATE]).is_some() {
                 Some(self.parse_literal_string()?)
             } else {
diff --git a/src/tokenizer.rs b/src/tokenizer.rs
index bc0f0ef..d33a7d8 100644
--- a/src/tokenizer.rs
+++ b/src/tokenizer.rs
@@ -2011,8 +2011,13 @@ impl<'a> Tokenizer<'a> {
                     num_consecutive_quotes = 0;
 
                     if let Some(next) = chars.peek() {
-                        if !self.unescape {
-                            // In no-escape mode, the given query has to be saved completely including backslashes.
+                        if !self.unescape
+                            || (self.dialect.ignores_wildcard_escapes()
+                                && (*next == '%' || *next == '_'))
+                        {
+                            // In no-escape mode, the given query has to be saved completely
+                            // including backslashes. Similarly, with ignore_like_wildcard_escapes,
+                            // the backslash is not stripped.
                             s.push(ch);
                             s.push(*next);
                             chars.next(); // consume next
@@ -3585,6 +3590,9 @@ mod tests {
             (r#"'\\a\\b\'c'"#, r#"\\a\\b\'c"#, r#"\a\b'c"#),
             (r#"'\'abcd'"#, r#"\'abcd"#, r#"'abcd"#),
             (r#"'''a''b'"#, r#"''a''b"#, r#"'a'b"#),
+            (r#"'\q'"#, r#"\q"#, r#"q"#),
+            (r#"'\%\_'"#, r#"\%\_"#, r#"%_"#),
+            (r#"'\\%\\_'"#, r#"\\%\\_"#, r#"\%\_"#),
         ] {
             let tokens = Tokenizer::new(&dialect, sql)
                 .with_unescape(false)
@@ -3618,6 +3626,16 @@ mod tests {
 
             compare(expected, tokens);
         }
+
+        // MySQL special case for LIKE escapes
+        for (sql, expected) in [(r#"'\%'"#, r#"\%"#), (r#"'\_'"#, r#"\_"#)] {
+            let dialect = MySqlDialect {};
+            let tokens = Tokenizer::new(&dialect, sql).tokenize().unwrap();
+
+            let expected = vec![Token::SingleQuotedString(expected.to_string())];
+
+            compare(expected, tokens);
+        }
     }
 
     #[test]


Test patch:
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 0a68d31..2c35c24 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -10387,15 +10387,8 @@ fn parse_with_recursion_limit() {
 
 #[test]
 fn parse_escaped_string_with_unescape() {
-    fn assert_mysql_query_value(sql: &str, quoted: &str) {
-        let stmt = TestedDialects::new(vec![
-            Box::new(MySqlDialect {}),
-            Box::new(BigQueryDialect {}),
-            Box::new(SnowflakeDialect {}),
-        ])
-        .one_statement_parses_to(sql, "");
-
-        match stmt {
+    fn assert_mysql_query_value(dialects: &TestedDialects, sql: &str, quoted: &str) {
+        match dialects.one_statement_parses_to(sql, "") {
             Statement::Query(query) => match *query.body {
                 SetExpr::Select(value) => {
                     let expr = expr_from_projection(only(&value.projection));
@@ -10411,17 +10404,38 @@ fn parse_escaped_string_with_unescape() {
             _ => unreachable!(),
         };
     }
+
+    let escaping_dialects =
+        &all_dialects_where(|dialect| dialect.supports_string_literal_backslash_escape());
+    let no_wildcard_exception = &all_dialects_where(|dialect| {
+        dialect.supports_string_literal_backslash_escape() && !dialect.ignores_wildcard_escapes()
+    });
+    let with_wildcard_exception = &all_dialects_where(|dialect| {
+        dialect.supports_string_literal_backslash_escape() && dialect.ignores_wildcard_escapes()
+    });
+
     let sql = r"SELECT 'I\'m fine'";
-    assert_mysql_query_value(sql, "I'm fine");
+    assert_mysql_query_value(escaping_dialects, sql, "I'm fine");
 
     let sql = r#"SELECT 'I''m fine'"#;
-    assert_mysql_query_value(sql, "I'm fine");
+    assert_mysql_query_value(escaping_dialects, sql, "I'm fine");
 
     let sql = r#"SELECT 'I\"m fine'"#;
-    assert_mysql_query_value(sql, "I\"m fine");
+    assert_mysql_query_value(escaping_dialects, sql, "I\"m fine");
 
     let sql = r"SELECT 'Testing: \0 \\ \% \_ \b \n \r \t \Z \a \h \ '";
-    assert_mysql_query_value(sql, "Testing: \0 \\ % _ \u{8} \n \r \t \u{1a} \u{7} h  ");
+    assert_mysql_query_value(
+        no_wildcard_exception,
+        sql,
+        "Testing: \0 \\ % _ \u{8} \n \r \t \u{1a} \u{7} h  ",
+    );
+
+    // check MySQL doesn't remove backslash from escaped LIKE wildcards
+    assert_mysql_query_value(
+        with_wildcard_exception,
+        sql,
+        "Testing: \0 \\ \\% \\_ \u{8} \n \r \t \u{1a} \u{7} h  ",
+    );
 }
 
 #[test]
@@ -14617,3 +14631,11 @@ fn parse_array_type_def_with_brackets() {
     dialects.verified_stmt("SELECT x::INT[]");
     dialects.verified_stmt("SELECT STRING_TO_ARRAY('1,2,3', ',')::INT[3]");
 }
+
+#[test]
+fn parse_set_names() {
+    let dialects = all_dialects_where(|d| d.supports_set_names());
+    dialects.verified_stmt("SET NAMES 'UTF8'");
+    dialects.verified_stmt("SET NAMES 'utf8'");
+    dialects.verified_stmt("SET NAMES UTF8 COLLATE bogus");
+}
diff --git a/tests/sqlparser_mssql.rs b/tests/sqlparser_mssql.rs
index ec565e5..3f313af 100644
--- a/tests/sqlparser_mssql.rs
+++ b/tests/sqlparser_mssql.rs
@@ -1620,6 +1620,22 @@ fn parse_create_table_with_valid_options() {
     }
 }
 
+#[test]
+fn parse_nested_slash_star_comment() {
+    let sql = r#"
+    select
+    /*
+       comment level 1
+       /*
+          comment level 2
+       */
+    */
+    1;
+    "#;
+    let canonical = "SELECT 1";
+    ms().one_statement_parses_to(sql, canonical);
+}
+
 #[test]
 fn parse_create_table_with_invalid_options() {
     let invalid_cases = vec![
diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs
index 15f79b4..8d89ce4 100644
--- a/tests/sqlparser_mysql.rs
+++ b/tests/sqlparser_mysql.rs
@@ -2627,6 +2627,17 @@ fn parse_rlike_and_regexp() {
     }
 }
 
+#[test]
+fn parse_like_with_escape() {
+    // verify backslash is not stripped for escaped wildcards
+    mysql().verified_only_select(r#"SELECT 'a\%c' LIKE 'a\%c'"#);
+    mysql().verified_only_select(r#"SELECT 'a\_c' LIKE 'a\_c'"#);
+    mysql().verified_only_select(r#"SELECT '%\_\%' LIKE '%\_\%'"#);
+    mysql().verified_only_select(r#"SELECT '\_\%' LIKE CONCAT('\_', '\%')"#);
+    mysql().verified_only_select(r#"SELECT 'a%c' LIKE 'a$%c' ESCAPE '$'"#);
+    mysql().verified_only_select(r#"SELECT 'a_c' LIKE 'a#_c' ESCAPE '#'"#);
+}
+
 #[test]
 fn parse_kill() {
     let stmt = mysql_and_generic().verified_stmt("KILL CONNECTION 5");
@@ -2685,7 +2696,7 @@ fn parse_set_names() {
     assert_eq!(
         stmt,
         Statement::SetNames {
-            charset_name: "utf8mb4".to_string(),
+            charset_name: "utf8mb4".into(),
             collation_name: None,
         }
     );
@@ -2694,7 +2705,7 @@ fn parse_set_names() {
     assert_eq!(
         stmt,
         Statement::SetNames {
-            charset_name: "utf8mb4".to_string(),
+            charset_name: "utf8mb4".into(),
             collation_name: Some("bogus".to_string()),
         }
     );
@@ -2705,7 +2716,7 @@ fn parse_set_names() {
     assert_eq!(
         stmt,
         vec![Statement::SetNames {
-            charset_name: "utf8mb4".to_string(),
+            charset_name: "utf8mb4".into(),
             collation_name: Some("bogus".to_string()),
         }]
     );