fix(query): fix comment display (#17615)

sundy-li · web-flow · commit ff841d133b7d · 2025-03-18T08:02:25.000+08:00
* fix(query): fix comment display

* update

* update

* update

* update

* update
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -557,7 +557,7 @@ tracing-subscriber = { version = "0.3.17", features = ["env-filter", "json", "va
 
 # Databend Integration Test
 quickcheck = "1.0"
-sqllogictest = "0.21.0"
+sqllogictest = "0.28.0"
 
 [workspace.lints.rust]
 async_fn_in_trait = "allow"
diff --git a/src/query/ast/src/ast/statements/data_mask.rs b/src/query/ast/src/ast/statements/data_mask.rs
@@ -18,6 +18,7 @@ use std::fmt::Formatter;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
+use crate::ast::quote::QuotedString;
 use crate::ast::CreateOption;
 use crate::ast::Expr;
 use crate::ast::TypeName;
@@ -68,7 +69,7 @@ impl Display for CreateDatamaskPolicyStmt {
             self.policy.return_type, self.policy.body
         )?;
         if let Some(comment) = &self.policy.comment {
-            write!(f, " COMMENT = '{}'", comment)?;
+            write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;
         }
 
         Ok(())
diff --git a/src/query/ast/src/ast/statements/dictionary.rs b/src/query/ast/src/ast/statements/dictionary.rs
@@ -20,6 +20,7 @@ use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
 use super::ShowLimit;
+use crate::ast::quote::QuotedString;
 use crate::ast::write_comma_separated_list;
 use crate::ast::write_dot_separated_list;
 use crate::ast::write_space_separated_string_map;
@@ -74,7 +75,7 @@ impl Display for CreateDictionaryStmt {
         write!(f, ")")?;
         write!(f, ")")?;
         if let Some(comment) = &self.comment {
-            write!(f, "COMMENT '{comment}' ")?;
+            write!(f, "COMMENT {} ", QuotedString(comment.clone(), '\''))?;
         }
         Ok(())
     }
diff --git a/src/query/ast/src/ast/statements/network_policy.rs b/src/query/ast/src/ast/statements/network_policy.rs
@@ -18,6 +18,7 @@ use std::fmt::Formatter;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
+use crate::ast::quote::QuotedString;
 use crate::ast::CreateOption;
 
 #[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
@@ -59,7 +60,7 @@ impl Display for CreateNetworkPolicyStmt {
             write!(f, ")")?;
         }
         if let Some(comment) = &self.comment {
-            write!(f, " COMMENT = '{}'", comment)?;
+            write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;
         }
 
         Ok(())
@@ -104,7 +105,7 @@ impl Display for AlterNetworkPolicyStmt {
             write!(f, ")")?;
         }
         if let Some(comment) = &self.comment {
-            write!(f, " COMMENT = '{}'", comment)?;
+            write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;
         }
 
         Ok(())
diff --git a/src/query/ast/src/ast/statements/password_policy.rs b/src/query/ast/src/ast/statements/password_policy.rs
@@ -18,6 +18,7 @@ use std::fmt::Formatter;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
+use crate::ast::quote::QuotedString;
 use crate::ast::CreateOption;
 
 #[derive(Debug, Clone, PartialEq, Drive, DriveMut)]
@@ -145,7 +146,7 @@ impl Display for PasswordSetOptions {
             write!(f, " PASSWORD_HISTORY = {}", history)?;
         }
         if let Some(comment) = &self.comment {
-            write!(f, " COMMENT = '{}'", comment)?;
+            write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;
         }
 
         Ok(())
diff --git a/src/query/ast/src/ast/statements/sequence.rs b/src/query/ast/src/ast/statements/sequence.rs
@@ -18,6 +18,7 @@ use std::fmt::Formatter;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
+use crate::ast::quote::QuotedString;
 use crate::ast::CreateOption;
 use crate::ast::Identifier;
 
@@ -40,7 +41,7 @@ impl Display for CreateSequenceStmt {
         }
         write!(f, " {}", self.sequence)?;
         if let Some(comment) = &self.comment {
-            write!(f, " COMMENT = '{}'", comment)?;
+            write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;
         }
         Ok(())
     }
diff --git a/src/query/ast/src/ast/statements/stream.rs b/src/query/ast/src/ast/statements/stream.rs
@@ -18,6 +18,7 @@ use std::fmt::Formatter;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
+use crate::ast::quote::QuotedString;
 use crate::ast::write_dot_separated_list;
 use crate::ast::CreateOption;
 use crate::ast::Identifier;
@@ -63,7 +64,7 @@ impl Display for CreateStreamStmt {
             write!(f, " APPEND_ONLY = false")?;
         }
         if let Some(comment) = &self.comment {
-            write!(f, " COMMENT = '{}'", comment)?;
+            write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;
         }
         Ok(())
     }
diff --git a/src/query/ast/src/ast/statements/task.rs b/src/query/ast/src/ast/statements/task.rs
@@ -19,6 +19,7 @@ use std::fmt::Formatter;
 use derive_visitor::Drive;
 use derive_visitor::DriveMut;
 
+use crate::ast::quote::QuotedString;
 use crate::ast::write_comma_separated_string_list;
 use crate::ast::write_comma_separated_string_map;
 use crate::ast::Expr;
@@ -298,7 +299,7 @@ impl Display for AlterTaskOptions {
                     write!(f, " SUSPEND_TASK_AFTER_NUM_FAILURES = {num}")?;
                 }
                 if let Some(comments) = comments {
-                    write!(f, " COMMENT = '{comments}'")?;
+                    write!(f, " COMMENT = {}", QuotedString(comments, '\''))?;
                 }
                 if let Some(error_integration) = error_integration {
                     write!(f, " ERROR_INTEGRATION = '{error_integration}'")?;
diff --git a/src/query/service/src/interpreters/interpreter_dictionary_show_create.rs b/src/query/service/src/interpreters/interpreter_dictionary_show_create.rs
@@ -15,6 +15,7 @@
 use std::sync::Arc;
 
 use databend_common_ast::ast::quote::display_ident;
+use databend_common_ast::ast::quote::QuotedString;
 use databend_common_ast::parser::Dialect;
 use databend_common_catalog::catalog::Catalog;
 use databend_common_exception::ErrorCode;
@@ -129,7 +130,7 @@ impl ShowCreateDictionaryInterpreter {
                 // compatibility: creating table in the old planner will not have `fields_comments`
                 let comment = field_comments
                     .get(&field.column_id)
-                    .and_then(|c| format!(" COMMENT '{}'", c).into())
+                    .and_then(|c| format!(" COMMENT {}", QuotedString(c, '\'')).into())
                     .unwrap_or_default();
 
                 let ident = display_ident(
diff --git a/src/query/service/src/interpreters/interpreter_table_show_create.rs b/src/query/service/src/interpreters/interpreter_table_show_create.rs
@@ -15,6 +15,7 @@
 use std::sync::Arc;
 
 use databend_common_ast::ast::quote::display_ident;
+use databend_common_ast::ast::quote::QuotedString;
 use databend_common_ast::parser::Dialect;
 use databend_common_catalog::catalog::Catalog;
 use databend_common_catalog::table::Table;
@@ -144,7 +145,6 @@ impl ShowCreateTableInterpreter {
         let engine = table.engine();
         let schema = table.schema();
         let field_comments = table.field_comments();
-        let n_fields = schema.fields().len();
         let sql_dialect = settings.sql_dialect;
         let force_quoted_ident = settings.force_quoted_ident;
         let quoted_ident_case_sensitive = settings.quoted_ident_case_sensitive;
@@ -204,18 +204,14 @@ impl ShowCreateTableInterpreter {
                     }
                     _ => "".to_string(),
                 };
-                // compatibility: creating table in the old planner will not have `fields_comments`
-                let comment = if field_comments.len() == n_fields && !field_comments[idx].is_empty()
+                let comment = if field_comments.len() == schema.fields().len()
+                    && !field_comments[idx].is_empty()
                 {
-                    // make the display more readable.
-                    // can not use debug print, will add double quote
-                    format!(
-                        " COMMENT '{}'",
-                        &field_comments[idx].as_str().replace('\'', "\\'")
-                    )
+                    format!(" COMMENT {}", QuotedString(&field_comments[idx], '\''))
                 } else {
                     "".to_string()
                 };
+
                 let ident = display_ident(
                     field.name(),
                     force_quoted_ident,
@@ -306,7 +302,13 @@ impl ShowCreateTableInterpreter {
         }
 
         if !table_info.meta.comment.is_empty() {
-            table_create_sql.push_str(format!(" COMMENT = '{}'", table_info.meta.comment).as_str());
+            table_create_sql.push_str(
+                format!(
+                    " COMMENT = {}",
+                    QuotedString(&table_info.meta.comment, '\'')
+                )
+                .as_str(),
+            );
         }
         Ok(table_create_sql)
     }
@@ -345,7 +347,7 @@ impl ShowCreateTableInterpreter {
 
         let comment = stream_table.get_table_info().meta.comment.clone();
         if !comment.is_empty() {
-            create_sql.push_str(format!(" COMMENT = '{}'", comment).as_str());
+            create_sql.push_str(format!(" COMMENT = {}", QuotedString(comment, '\'')).as_str());
         }
         Ok(create_sql)
     }
diff --git a/src/query/sql/src/planner/binder/ddl/table.rs b/src/query/sql/src/planner/binder/ddl/table.rs
@@ -1570,7 +1570,6 @@ impl Binder {
             let name = normalize_identifier(&column.name, &self.name_resolution_ctx).name;
             let schema_data_type = resolve_type_name(&column.data_type, not_null)?;
             fields_comments.push(column.comment.clone().unwrap_or_default());
-
             let mut field = TableField::new(&name, schema_data_type.clone());
             if let Some(expr) = &column.expr {
                 match expr {
diff --git a/src/query/storages/system/src/columns_table.rs b/src/query/storages/system/src/columns_table.rs
@@ -207,17 +207,17 @@ impl ColumnsTable {
                     _ => {
                         let schema = table.schema();
                         let field_comments = table.field_comments();
-                        let n_fields = schema.fields().len();
+
                         for (idx, field) in schema.fields().iter().enumerate() {
-                            // compatibility: creating table in the old planner will not have `fields_comments`
-                            let comment = if field_comments.len() == n_fields
+                            let comment = if field_comments.len() == schema.fields.len()
                                 && !field_comments[idx].is_empty()
                             {
                                 // can not use debug print, will add double quote
-                                format!("{}", &field_comments[idx].as_str().replace('\'', "\\'"))
+                                field_comments[idx].clone()
                             } else {
                                 "".to_string()
                             };
+
                             rows.push((
                                 database.clone(),
                                 table.name().into(),
diff --git a/tests/sqllogictests/src/main.rs b/tests/sqllogictests/src/main.rs
@@ -102,6 +102,8 @@ impl sqllogictest::AsyncDB for Databend {
         self.client.query(sql).await
     }
 
+    async fn shutdown(&mut self) {}
+
     fn engine_name(&self) -> &str {
         self.client.engine_name()
     }
@@ -311,6 +313,7 @@ async fn run_suits(args: SqlLogicTestArgs, client_type: ClientType) -> Result<()
                     file.unwrap().path(),
                     col_separator,
                     validator,
+                    sqllogictest::default_normalizer,
                     column_validator,
                 )
                 .await
diff --git a/tests/sqllogictests/suites/base/06_show/06_0001_show_create_table.test b/tests/sqllogictests/suites/base/06_show/06_0001_show_create_table.test
diff --git a/tests/suites/0_stateless/02_ddl/02_0001_show_create_table.result b/tests/suites/0_stateless/02_ddl/02_0001_show_create_table.result
diff --git a/tests/suites/0_stateless/02_ddl/02_0001_show_create_table.sh b/tests/suites/0_stateless/02_ddl/02_0001_show_create_table.sh

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ use std::fmt::Formatter;`
`18`	`18`	`use derive_visitor::Drive;`
`19`	`19`	`use derive_visitor::DriveMut;`
`20`	`20`
	`21`	`+use crate::ast::quote::QuotedString;`
`21`	`22`	`use crate::ast::CreateOption;`
`22`	`23`
`23`	`24`	`#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]`
`@@ -59,7 +60,7 @@ impl Display for CreateNetworkPolicyStmt {`
`59`	`60`	`write!(f, ")")?;`
`60`	`61`	`}`
`61`	`62`	`if let Some(comment) = &self.comment {`
`62`		`- write!(f, " COMMENT = '{}'", comment)?;`
	`63`	`+ write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;`
`63`	`64`	`}`
`64`	`65`
`65`	`66`	`Ok(())`
`@@ -104,7 +105,7 @@ impl Display for AlterNetworkPolicyStmt {`
`104`	`105`	`write!(f, ")")?;`
`105`	`106`	`}`
`106`	`107`	`if let Some(comment) = &self.comment {`
`107`		`- write!(f, " COMMENT = '{}'", comment)?;`
	`108`	`+ write!(f, " COMMENT = {}", QuotedString(comment, '\''))?;`
`108`	`109`	`}`
`109`	`110`
`110`	`111`	`Ok(())`