From 4c52327704aa7314824e9a9a64635bfac7bbe812 Mon Sep 17 00:00:00 2001
From: Elia Perantoni <eper@wayfare.ai>
Date: Wed, 25 Jun 2025 15:15:47 +0200
Subject: [PATCH] feat: pr feedback

---
 src/ast/ddl.rs               | 11 +++++++----
 src/ast/spans.rs             | 16 ++++++++--------
 src/dialect/mod.rs           |  4 ++++
 src/dialect/snowflake.rs     |  4 ++++
 src/parser/mod.rs            | 16 +++++++---------
 tests/sqlparser_common.rs    |  2 +-
 tests/sqlparser_snowflake.rs |  2 +-
 7 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs
index 0258b759..d2863c3a 100644
--- a/src/ast/ddl.rs
+++ b/src/ast/ddl.rs
@@ -1453,10 +1453,13 @@ impl fmt::Display for ViewColumnDef {
             write!(f, " {}", data_type)?;
         }
         if let Some(options) = self.options.as_ref() {
-            if matches!(options, ColumnOptions::CommaSeparated(_)) {
-                write!(f, " {}", display_comma_separated(options.as_slice()))?;
-            } else {
-                write!(f, " {}", display_separated(options.as_slice(), " "))?
+            match options {
+                ColumnOptions::CommaSeparated(column_options) => {
+                    write!(f, " {}", display_comma_separated(column_options.as_slice()))?;
+                }
+                ColumnOptions::SpaceSeparated(column_options) => {
+                    write!(f, " {}", display_separated(column_options.as_slice(), " "))?
+                }
             }
         }
         Ok(())
diff --git a/src/ast/spans.rs b/src/ast/spans.rs
index 07d41421..78ed772b 100644
--- a/src/ast/spans.rs
+++ b/src/ast/spans.rs
@@ -15,7 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-use crate::ast::query::SelectItemQualifiedWildcardKind;
+use crate::ast::{query::SelectItemQualifiedWildcardKind, ColumnOptions};
 use core::iter;
 
 use crate::tokenizer::Span;
@@ -991,13 +991,13 @@ impl Spanned for ViewColumnDef {
             options,
         } = self;
 
-        union_spans(
-            core::iter::once(name.span).chain(
-                options
-                    .iter()
-                    .flat_map(|i| i.as_slice().iter().map(|k| k.span())),
-            ),
-        )
+        name.span.union_opt(&options.as_ref().map(|o| o.span()))
+    }
+}
+
+impl Spanned for ColumnOptions {
+    fn span(&self) -> Span {
+        union_spans(self.as_slice().iter().map(|i| i.span()))
     }
 }
 
diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs
index a4c899e6..bc92948d 100644
--- a/src/dialect/mod.rs
+++ b/src/dialect/mod.rs
@@ -1028,6 +1028,10 @@ pub trait Dialect: Debug + Any {
     fn supports_set_names(&self) -> bool {
         false
     }
+
+    fn supports_space_separated_column_options(&self) -> bool {
+        false
+    }
 }
 
 /// This represents the operators for which precedence must be defined
diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs
index 66e04ac2..5ebb7e37 100644
--- a/src/dialect/snowflake.rs
+++ b/src/dialect/snowflake.rs
@@ -356,6 +356,10 @@ impl Dialect for SnowflakeDialect {
     fn get_reserved_keywords_for_select_item_operator(&self) -> &[Keyword] {
         &RESERVED_KEYWORDS_FOR_SELECT_ITEM_OPERATOR
     }
+
+    fn supports_space_separated_column_options(&self) -> bool {
+        true
+    }
 }
 
 fn parse_file_staging_command(kw: Keyword, parser: &mut Parser) -> Result<Statement, ParserError> {
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index e7b767f8..44ba0bb7 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -22,7 +22,6 @@ use alloc::{
 };
 use core::{
     fmt::{self, Display},
-    ops::Not,
     str::FromStr,
 };
 use helpers::attached_token::AttachedToken;
@@ -10603,14 +10602,13 @@ impl<'a> Parser<'a> {
                 break;
             }
         }
-        Ok(options
-            .is_empty()
-            .not()
-            .then_some(if dialect_of!(self is SnowflakeDialect) {
-                ColumnOptions::SpaceSeparated(options)
-            } else {
-                ColumnOptions::CommaSeparated(options)
-            }))
+        if options.is_empty() {
+            Ok(None)
+        } else if self.dialect.supports_space_separated_column_options() {
+            Ok(Some(ColumnOptions::SpaceSeparated(options)))
+        } else {
+            Ok(Some(ColumnOptions::CommaSeparated(options)))
+        }
     }
 
     /// Parses a parenthesized comma-separated list of unqualified, possibly quoted identifiers.
diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs
index 089dbc90..3c20f82a 100644
--- a/tests/sqlparser_common.rs
+++ b/tests/sqlparser_common.rs
@@ -7964,7 +7964,7 @@ fn parse_create_view_with_columns() {
     let sql = "CREATE VIEW v (has, cols) AS SELECT 1, 2";
     // TODO: why does this fail for ClickHouseDialect? (#1449)
     // match all_dialects().verified_stmt(sql) {
-    match all_dialects_where(|d| !d.is::<ClickHouseDialect>()).verified_stmt(sql) {
+    match all_dialects_except(|d| d.is::<ClickHouseDialect>()).verified_stmt(sql) {
         Statement::CreateView {
             or_alter,
             name,
diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs
index 48edade4..8b3988d9 100644
--- a/tests/sqlparser_snowflake.rs
+++ b/tests/sqlparser_snowflake.rs
@@ -4169,7 +4169,7 @@ fn test_snowflake_fetch_clause_syntax() {
 }
 
 #[test]
-fn test_snowflake_create_view_with_tag() {
+fn test_snowflake_create_view_with_multiple_column_options() {
     let create_view_with_tag =
         r#"CREATE VIEW X (COL WITH TAG (pii='email') COMMENT 'foobar') AS SELECT * FROM Y"#;
     snowflake().verified_stmt(create_view_with_tag);