fix: allow comma seperated connection option (#13100)

* fix: allow comma seperated connection option

* address comment

* improve display

* fix

* fix

* fix

src/query/ast/src/ast/ast.rs

@@ -114,7 +114,7 @@ pub(crate) fn write_comma_separated_list(
 }
 
 /// Write input items into `'a', 'b', 'c'`
-pub(crate) fn write_quoted_comma_separated_list(
+pub(crate) fn write_comma_separated_quoted_list(
     f: &mut Formatter<'_>,
     items: impl IntoIterator<Item = impl Display>,
 ) -> std::fmt::Result {
@@ -127,16 +127,30 @@ pub(crate) fn write_quoted_comma_separated_list(
     Ok(())
 }
 
-/// Write input map items into `field_a=x field_b=y`
-pub(crate) fn write_space_separated_map(
+/// Write input map items into `field_a=x, field_b=y`
+pub(crate) fn write_comma_separated_map(
     f: &mut Formatter<'_>,
     items: impl IntoIterator<Item = (impl Display, impl Display)>,
 ) -> std::fmt::Result {
     for (i, (k, v)) in items.into_iter().enumerate() {
         if i > 0 {
-            write!(f, " ")?;
+            write!(f, ", ")?;
         }
         write!(f, "{k}='{v}'")?;
     }
     Ok(())
 }
+
+/// Write input map items into `field_a=>x, field_b=>y`
+pub(crate) fn write_comma_separated_arrow_map(
+    f: &mut Formatter<'_>,
+    items: impl IntoIterator<Item = (impl Display, impl Display)>,
+) -> std::fmt::Result {
+    for (i, (k, v)) in items.into_iter().enumerate() {
+        if i > 0 {
+            write!(f, ", ")?;
+        }
+        write!(f, "{k}=>'{v}'")?;
+    }
+    Ok(())
+}

src/query/ast/src/ast/statements/catalog.rs

@@ -19,6 +19,7 @@ use std::fmt::Formatter;
 use common_meta_app::schema::CatalogType;
 
 use super::ShowLimit;
+use crate::ast::write_comma_separated_map;
 use crate::ast::Identifier;
 
 #[derive(Debug, Clone, PartialEq)]
@@ -64,10 +65,8 @@ impl Display for CreateCatalogStmt {
         }
         write!(f, " {}", self.catalog_name)?;
         write!(f, " TYPE='{}'", self.catalog_type)?;
-        write!(f, " CONNECTION = (")?;
-        for (k, v) in self.catalog_options.iter() {
-            write!(f, " {}='{}'", k, v)?;
-        }
+        write!(f, " CONNECTION = ( ")?;
+        write_comma_separated_map(f, &self.catalog_options)?;
         write!(f, " )")
     }
 }

src/query/ast/src/ast/statements/copy.rs

@@ -23,8 +23,8 @@ use std::io::Result;
 use itertools::Itertools;
 use url::Url;
 
-use crate::ast::write_quoted_comma_separated_list;
-use crate::ast::write_space_separated_map;
+use crate::ast::write_comma_separated_map;
+use crate::ast::write_comma_separated_quoted_list;
 use crate::ast::Hint;
 use crate::ast::Identifier;
 use crate::ast::Query;
@@ -88,7 +88,7 @@ impl Display for CopyStmt {
 
         if let Some(files) = &self.files {
             write!(f, " FILES = (")?;
-            write_quoted_comma_separated_list(f, files)?;
+            write_comma_separated_quoted_list(f, files)?;
             write!(f, " )")?;
         }
 
@@ -98,10 +98,8 @@ impl Display for CopyStmt {
 
         if !self.file_format.is_empty() {
             write!(f, " FILE_FORMAT = (")?;
-            for (k, v) in self.file_format.iter() {
-                write!(f, " {} = '{}'", k, v)?;
-            }
-            write!(f, " )")?;
+            write_comma_separated_map(f, &self.file_format)?;
+            write!(f, ")")?;
         }
 
         if !self.validation_mode.is_empty() {
@@ -256,7 +254,7 @@ impl Display for Connection {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         if !self.conns.is_empty() {
             write!(f, " CONNECTION = ( ")?;
-            write_space_separated_map(f, &self.conns)?;
+            write_comma_separated_map(f, &self.conns)?;
             write!(f, " )")?;
         }
         Ok(())

src/query/ast/src/ast/statements/insert.rs

@@ -17,6 +17,7 @@ use std::fmt::Display;
 use std::fmt::Formatter;
 
 use crate::ast::write_comma_separated_list;
+use crate::ast::write_comma_separated_map;
 use crate::ast::write_dot_separated_list;
 use crate::ast::Hint;
 use crate::ast::Identifier;
@@ -95,9 +96,7 @@ impl Display for InsertSource {
                 start: _,
             } => {
                 write!(f, " FILE_FORMAT = (")?;
-                for (k, v) in settings.iter() {
-                    write!(f, " {} = '{}'", k, v)?;
-                }
+                write_comma_separated_map(f, settings)?;
                 write!(f, " )")?;
                 write!(
                     f,

src/query/ast/src/ast/statements/merge_into.rs

@@ -21,6 +21,7 @@ use common_exception::Result;
 
 use super::Hint;
 use crate::ast::write_comma_separated_list;
+use crate::ast::write_comma_separated_map;
 use crate::ast::write_dot_separated_list;
 use crate::ast::Expr;
 use crate::ast::Identifier;
@@ -257,9 +258,7 @@ impl Display for MergeSource {
                 start: _,
             } => {
                 write!(f, " FILE_FORMAT = (")?;
-                for (k, v) in settings.iter() {
-                    write!(f, " {} = '{}'", k, v)?;
-                }
+                write_comma_separated_map(f, settings)?;
                 write!(f, " )")?;
                 write!(
                     f,

src/query/ast/src/ast/statements/stage.rs

@@ -17,6 +17,9 @@ use std::default::Default;
 use std::fmt::Display;
 use std::fmt::Formatter;
 
+use crate::ast::write_comma_separated_arrow_map;
+use crate::ast::write_comma_separated_map;
+use crate::ast::write_comma_separated_quoted_list;
 use crate::ast::UriLocation;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -48,9 +51,7 @@ impl Display for CreateStageStmt {
 
         if !self.file_format_options.is_empty() {
             write!(f, " FILE_FORMAT = (")?;
-            for (k, v) in self.file_format_options.iter() {
-                write!(f, " {} = '{}'", k, v)?;
-            }
+            write_comma_separated_map(f, &self.file_format_options)?;
             write!(f, " )")?;
         }
 
@@ -118,35 +119,24 @@ impl Display for SelectStageOptions {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(f, " (")?;
 
-        let mut output: Vec<String> = vec![];
-        if let Some(files) = self.files.clone() {
-            let files = files
-                .iter()
-                .map(|x| format!("'{}'", x))
-                .collect::<Vec<String>>();
-            let files = files.join(",");
-            let files = format!("FILES => ({})", files);
-            output.push(files);
+        if let Some(files) = self.files.as_ref() {
+            write!(f, " FILES => (")?;
+            write_comma_separated_quoted_list(f, files)?;
+            write!(f, "),")?;
         }
 
-        if let Some(file_format) = self.file_format.clone() {
-            let file_format = format!("FILE_FORMAT => '{}'", file_format);
-            output.push(file_format);
+        if let Some(file_format) = self.file_format.as_ref() {
+            write!(f, " FILE_FORMAT => '{}',", file_format)?;
         }
 
-        if let Some(pattern) = self.pattern.clone() {
-            let pattern = format!("PATTERN => '{}'", pattern);
-            output.push(pattern);
+        if let Some(pattern) = self.pattern.as_ref() {
+            write!(f, " PATTERN => '{}',", pattern)?;
         }
 
-        if !self.connection.is_empty() {
-            for (k, v) in self.connection.iter() {
-                output.push(format!(" {} => '{}'", k, v));
-            }
-        }
+        write_comma_separated_arrow_map(f, &self.connection)?;
+
+        write!(f, " )")?;
 
-        let output = output.join(",");
-        write!(f, "{output})")?;
         Ok(())
     }
 }

src/query/ast/src/ast/statements/table.rs

@@ -19,8 +19,8 @@ use std::format;
 
 use crate::ast::statements::show::ShowLimit;
 use crate::ast::write_comma_separated_list;
+use crate::ast::write_comma_separated_map;
 use crate::ast::write_dot_separated_list;
-use crate::ast::write_space_separated_map;
 use crate::ast::Expr;
 use crate::ast::Identifier;
 use crate::ast::Query;
@@ -167,7 +167,7 @@ impl Display for CreateTableStmt {
         }
 
         // Format table options
-        write_space_separated_map(f, self.table_options.iter())?;
+        write_comma_separated_map(f, &self.table_options)?;
         if let Some(as_query) = &self.as_query {
             write!(f, " AS {as_query}")?;
         }
@@ -354,34 +354,34 @@ impl Display for AlterTableAction {
     fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
         match self {
             AlterTableAction::SetOptions { set_options } => {
-                write!(f, "SET OPTIONS: ").expect("Set Options Write Error ");
-                write_space_separated_map(f, set_options.iter())
+                write!(f, "SET OPTIONS (")?;
+                write_comma_separated_map(f, set_options)?;
+                write!(f, ")")?;
             }
             AlterTableAction::RenameTable { new_table } => {
-                write!(f, "RENAME TO {new_table}")
+                write!(f, "RENAME TO {new_table}")?;
             }
             AlterTableAction::RenameColumn {
                 old_column,
                 new_column,
             } => {
-                write!(f, "RENAME COLUMN {old_column} TO {new_column}")
+                write!(f, "RENAME COLUMN {old_column} TO {new_column}")?;
             }
             AlterTableAction::AddColumn { column, option } => {
                 write!(f, "ADD COLUMN {column}{option}")?;
-                Ok(())
             }
             AlterTableAction::ModifyColumn { action } => {
-                write!(f, "MODIFY COLUMN {action}")
+                write!(f, "MODIFY COLUMN {action}")?;
             }
             AlterTableAction::DropColumn { column } => {
-                write!(f, "DROP COLUMN {column}")
+                write!(f, "DROP COLUMN {column}")?;
             }
             AlterTableAction::AlterTableClusterKey { cluster_by } => {
                 write!(f, "CLUSTER BY ")?;
-                write_comma_separated_list(f, cluster_by)
+                write_comma_separated_list(f, cluster_by)?;
             }
             AlterTableAction::DropTableClusterKey => {
-                write!(f, "DROP CLUSTER KEY")
+                write!(f, "DROP CLUSTER KEY")?;
             }
             AlterTableAction::ReclusterTable {
                 is_final,
@@ -398,13 +398,12 @@ impl Display for AlterTableAction {
                 if let Some(limit) = limit {
                     write!(f, " LIMIT {limit}")?;
                 }
-                Ok(())
             }
             AlterTableAction::RevertTo { point } => {
                 write!(f, "REVERT TO {}", point)?;
-                Ok(())
             }
-        }
+        };
+        Ok(())
     }
 }