From 33963675d75c11e64e423fd79ab998193dbb4286 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Fri, 24 May 2024 16:21:50 +0200 Subject: [PATCH 1/4] Add support for view comments for Snowflake Snowflake supports adding a comment when creating a view, see https://docs.snowflake.com/en/sql-reference/sql/create-view#syntax. --- src/ast/mod.rs | 7 +++++ src/parser/mod.rs | 16 +++++++++++ tests/sqlparser_bigquery.rs | 2 ++ tests/sqlparser_common.rs | 53 +++++++++++++++++++++++++++++++++++++ tests/sqlparser_sqlite.rs | 2 ++ 5 files changed, 80 insertions(+) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index c9de747c7..aa712a00d 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -1958,6 +1958,9 @@ pub enum Statement { query: Box, options: CreateTableOptions, cluster_by: Vec, + /// Snowflake: Views can have comments in Snowflake. + /// + comment: Option, /// if true, has RedShift [`WITH NO SCHEMA BINDING`] clause with_no_schema_binding: bool, /// if true, has SQLite `IF NOT EXISTS` clause @@ -3226,6 +3229,7 @@ impl fmt::Display for Statement { materialized, options, cluster_by, + comment, with_no_schema_binding, if_not_exists, temporary, @@ -3239,6 +3243,9 @@ impl fmt::Display for Statement { temporary = if *temporary { "TEMPORARY " } else { "" }, if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" } )?; + if let Some(comment) = comment { + write!(f, " COMMENT = '{comment}'")?; + } if matches!(options, CreateTableOptions::With(_)) { write!(f, " {options}")?; } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index f88aefd10..052d52e17 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -3903,6 +3903,21 @@ impl<'a> Parser<'a> { }; } + let comment = if dialect_of!(self is SnowflakeDialect) { + if self.parse_keyword(Keyword::COMMENT) { + let _ = self.consume_token(&Token::Eq); + let next_token = self.next_token(); + match next_token.token { + Token::SingleQuotedString(str) => Some(str), + _ => self.expected("comment", next_token)?, + } + } else { + None + } + } else { + None + }; + self.expect_keyword(Keyword::AS)?; let query = self.parse_boxed_query()?; // Optional `WITH [ CASCADED | LOCAL ] CHECK OPTION` is widely supported here. @@ -3923,6 +3938,7 @@ impl<'a> Parser<'a> { or_replace, options, cluster_by, + comment, with_no_schema_binding, if_not_exists, temporary, diff --git a/tests/sqlparser_bigquery.rs b/tests/sqlparser_bigquery.rs index 179755e0c..d03040c4c 100644 --- a/tests/sqlparser_bigquery.rs +++ b/tests/sqlparser_bigquery.rs @@ -309,6 +309,7 @@ fn parse_create_view_if_not_exists() { materialized, options, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -320,6 +321,7 @@ fn parse_create_view_if_not_exists() { assert!(!or_replace); assert_eq!(options, CreateTableOptions::None); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(if_not_exists); assert!(!temporary); diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index f8b7d0265..28d3458e2 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -6251,6 +6251,7 @@ fn parse_create_view() { materialized, options, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6262,6 +6263,7 @@ fn parse_create_view() { assert!(!or_replace); assert_eq!(options, CreateTableOptions::None); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(!if_not_exists); assert!(!temporary); @@ -6305,6 +6307,7 @@ fn parse_create_view_with_columns() { query, materialized, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6325,6 +6328,7 @@ fn parse_create_view_with_columns() { assert!(!materialized); assert!(!or_replace); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(!if_not_exists); assert!(!temporary); @@ -6345,6 +6349,7 @@ fn parse_create_view_temporary() { materialized, options, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6356,6 +6361,7 @@ fn parse_create_view_temporary() { assert!(!or_replace); assert_eq!(options, CreateTableOptions::None); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(!if_not_exists); assert!(temporary); @@ -6376,6 +6382,7 @@ fn parse_create_or_replace_view() { query, materialized, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6387,6 +6394,46 @@ fn parse_create_or_replace_view() { assert!(!materialized); assert!(or_replace); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); + assert!(!late_binding); + assert!(!if_not_exists); + assert!(!temporary); + } + _ => unreachable!(), + } +} + +#[test] +fn parse_create_or_replace_with_comment_for_snowflake() { + let sql = "CREATE OR REPLACE VIEW v COMMENT = 'hello, world' AS SELECT 1"; + let dialect = test_utils::TestedDialects { + dialects: vec![Box::new(SnowflakeDialect {}) as Box], + options: None, + }; + + match dialect.verified_stmt(sql) { + Statement::CreateView { + name, + columns, + or_replace, + options, + query, + materialized, + cluster_by, + comment, + with_no_schema_binding: late_binding, + if_not_exists, + temporary, + } => { + assert_eq!("v", name.to_string()); + assert_eq!(columns, vec![]); + assert_eq!(options, CreateTableOptions::None); + assert_eq!("SELECT 1", query.to_string()); + assert!(!materialized); + assert!(or_replace); + assert_eq!(cluster_by, vec![]); + assert!(comment.is_some()); + assert_eq!(comment.expect("expected comment"), "hello, world"); assert!(!late_binding); assert!(!if_not_exists); assert!(!temporary); @@ -6411,6 +6458,7 @@ fn parse_create_or_replace_materialized_view() { query, materialized, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6422,6 +6470,7 @@ fn parse_create_or_replace_materialized_view() { assert!(materialized); assert!(or_replace); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(!if_not_exists); assert!(!temporary); @@ -6442,6 +6491,7 @@ fn parse_create_materialized_view() { materialized, options, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6453,6 +6503,7 @@ fn parse_create_materialized_view() { assert_eq!(options, CreateTableOptions::None); assert!(!or_replace); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(!if_not_exists); assert!(!temporary); @@ -6473,6 +6524,7 @@ fn parse_create_materialized_view_with_cluster_by() { materialized, options, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -6484,6 +6536,7 @@ fn parse_create_materialized_view_with_cluster_by() { assert_eq!(options, CreateTableOptions::None); assert!(!or_replace); assert_eq!(cluster_by, vec![Ident::new("foo")]); + assert!(comment.is_none()); assert!(!late_binding); assert!(!if_not_exists); assert!(!temporary); diff --git a/tests/sqlparser_sqlite.rs b/tests/sqlparser_sqlite.rs index 5742754c0..fe5346f14 100644 --- a/tests/sqlparser_sqlite.rs +++ b/tests/sqlparser_sqlite.rs @@ -167,6 +167,7 @@ fn parse_create_view_temporary_if_not_exists() { materialized, options, cluster_by, + comment, with_no_schema_binding: late_binding, if_not_exists, temporary, @@ -178,6 +179,7 @@ fn parse_create_view_temporary_if_not_exists() { assert!(!or_replace); assert_eq!(options, CreateTableOptions::None); assert_eq!(cluster_by, vec![]); + assert!(comment.is_none()); assert!(!late_binding); assert!(if_not_exists); assert!(temporary); From 2c5c1819876615812d95889d8aab777d2af77940 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Sun, 26 May 2024 21:49:19 +0200 Subject: [PATCH 2/4] Address MR feedback - Support `GenericDialect` as well - Inline keyword match - Use `expect_token` instead of `consume_token` for the equal sign - Fix faulty expect text - Move test to `sqlparser_snowflake.rs` - Add negative test for missing equal sign --- src/parser/mod.rs | 18 ++++++------- tests/sqlparser_common.rs | 39 --------------------------- tests/sqlparser_snowflake.rs | 52 +++++++++++++++++++++++++++++++++++- 3 files changed, 59 insertions(+), 50 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 052d52e17..f9e997e91 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -3903,16 +3903,14 @@ impl<'a> Parser<'a> { }; } - let comment = if dialect_of!(self is SnowflakeDialect) { - if self.parse_keyword(Keyword::COMMENT) { - let _ = self.consume_token(&Token::Eq); - let next_token = self.next_token(); - match next_token.token { - Token::SingleQuotedString(str) => Some(str), - _ => self.expected("comment", next_token)?, - } - } else { - None + let comment = if dialect_of!(self is SnowflakeDialect | GenericDialect) + && self.parse_keyword(Keyword::COMMENT) + { + self.expect_token(&Token::Eq)?; + let next_token = self.next_token(); + match next_token.token { + Token::SingleQuotedString(str) => Some(str), + _ => self.expected("string literal", next_token)?, } } else { None diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 28d3458e2..36322d52e 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -6403,45 +6403,6 @@ fn parse_create_or_replace_view() { } } -#[test] -fn parse_create_or_replace_with_comment_for_snowflake() { - let sql = "CREATE OR REPLACE VIEW v COMMENT = 'hello, world' AS SELECT 1"; - let dialect = test_utils::TestedDialects { - dialects: vec![Box::new(SnowflakeDialect {}) as Box], - options: None, - }; - - match dialect.verified_stmt(sql) { - Statement::CreateView { - name, - columns, - or_replace, - options, - query, - materialized, - cluster_by, - comment, - with_no_schema_binding: late_binding, - if_not_exists, - temporary, - } => { - assert_eq!("v", name.to_string()); - assert_eq!(columns, vec![]); - assert_eq!(options, CreateTableOptions::None); - assert_eq!("SELECT 1", query.to_string()); - assert!(!materialized); - assert!(or_replace); - assert_eq!(cluster_by, vec![]); - assert!(comment.is_some()); - assert_eq!(comment.expect("expected comment"), "hello, world"); - assert!(!late_binding); - assert!(!if_not_exists); - assert!(!temporary); - } - _ => unreachable!(), - } -} - #[test] fn parse_create_or_replace_materialized_view() { // Supported in BigQuery (Beta) diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index 30f2cc601..bf3eb9fac 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -18,7 +18,7 @@ use sqlparser::ast::helpers::stmt_data_loading::{ DataLoadingOption, DataLoadingOptionType, StageLoadSelectItem, }; use sqlparser::ast::*; -use sqlparser::dialect::{GenericDialect, SnowflakeDialect}; +use sqlparser::dialect::{Dialect, GenericDialect, SnowflakeDialect}; use sqlparser::parser::{ParserError, ParserOptions}; use sqlparser::tokenizer::*; use test_utils::*; @@ -91,6 +91,56 @@ fn test_snowflake_single_line_tokenize() { assert_eq!(expected, tokens); } +#[test] +fn parse_sf_create_or_replace_view_with_comment_missing_equal() { + assert!(snowflake_and_generic() + .parse_sql_statements("CREATE OR REPLACE VIEW v COMMENT = 'hello, world' AS SELECT 1") + .is_ok()); + + assert!(snowflake_and_generic() + .parse_sql_statements("CREATE OR REPLACE VIEW v COMMENT 'hello, world' AS SELECT 1") + .is_err()); +} + +#[test] +fn parse_sf_create_or_replace_with_comment_for_snowflake() { + let sql = "CREATE OR REPLACE VIEW v COMMENT = 'hello, world' AS SELECT 1"; + let dialect = test_utils::TestedDialects { + dialects: vec![Box::new(SnowflakeDialect {}) as Box], + options: None, + }; + + match dialect.verified_stmt(sql) { + Statement::CreateView { + name, + columns, + or_replace, + options, + query, + materialized, + cluster_by, + comment, + with_no_schema_binding: late_binding, + if_not_exists, + temporary, + } => { + assert_eq!("v", name.to_string()); + assert_eq!(columns, vec![]); + assert_eq!(options, CreateTableOptions::None); + assert_eq!("SELECT 1", query.to_string()); + assert!(!materialized); + assert!(or_replace); + assert_eq!(cluster_by, vec![]); + assert!(comment.is_some()); + assert_eq!(comment.expect("expected comment"), "hello, world"); + assert!(!late_binding); + assert!(!if_not_exists); + assert!(!temporary); + } + _ => unreachable!(), + } +} + #[test] fn test_sf_derived_table_in_parenthesis() { // Nesting a subquery in an extra set of parentheses is non-standard, From 074a708635d26bb32684c478e041f6a3048c4fc8 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Tue, 28 May 2024 08:28:54 +0200 Subject: [PATCH 3/4] Update src/ast/mod.rs Co-authored-by: Joey Hain --- src/ast/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index aa712a00d..251967718 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -3244,7 +3244,7 @@ impl fmt::Display for Statement { if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" } )?; if let Some(comment) = comment { - write!(f, " COMMENT = '{comment}'")?; + write!(f, " COMMENT = '{}'", value::escape_single_quote_string(comment))?; } if matches!(options, CreateTableOptions::With(_)) { write!(f, " {options}")?; From 3e0fd5a6614367414d95938594d70d6f78f591e7 Mon Sep 17 00:00:00 2001 From: Simon Sawert Date: Tue, 28 May 2024 10:25:38 +0200 Subject: [PATCH 4/4] Fix formatting --- src/ast/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 251967718..b44a341bc 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -3244,7 +3244,11 @@ impl fmt::Display for Statement { if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" } )?; if let Some(comment) = comment { - write!(f, " COMMENT = '{}'", value::escape_single_quote_string(comment))?; + write!( + f, + " COMMENT = '{}'", + value::escape_single_quote_string(comment) + )?; } if matches!(options, CreateTableOptions::With(_)) { write!(f, " {options}")?;