From 1858c13cab9443616c0db495e505d98226ddee08 Mon Sep 17 00:00:00 2001 From: sabir Date: Thu, 2 Jul 2026 15:49:22 +0200 Subject: [PATCH 1/4] Snowflake: Add EXTERNAL VOLUME DDL Snowflake's external-volume statements did not parse. This adds CREATE [OR REPLACE] EXTERNAL VOLUME [IF NOT EXISTS], ALTER, DROP, DESCRIBE/DESC and SHOW EXTERNAL VOLUMES. Example that failed before: CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = ( (NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/path/')) Storage locations accept STORAGE_BASE_URL, STORAGE_AWS_ROLE_ARN, STORAGE_AWS_EXTERNAL_ID and ENCRYPTION in any order after NAME and STORAGE_PROVIDER, matching real Snowflake; required fields and duplicates are validated after parsing. All statements round-trip. --- src/ast/mod.rs | 224 +++++++++++++ src/ast/spans.rs | 5 + src/dialect/snowflake.rs | 250 ++++++++++++++- src/keywords.rs | 9 + tests/sqlparser_snowflake.rs | 592 +++++++++++++++++++++++++++++++++++ 5 files changed, 1072 insertions(+), 8 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 88883cfbb..540c12488 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -4536,6 +4536,58 @@ pub enum Statement { comment: Option, }, /// ```sql + /// CREATE [OR REPLACE] EXTERNAL VOLUME [IF NOT EXISTS] + /// ``` + /// See + CreateExternalVolume { + /// `OR REPLACE` flag. + or_replace: bool, + /// `IF NOT EXISTS` flag. + if_not_exists: bool, + /// External volume name. + name: ObjectName, + /// Storage locations. + storage_locations: Vec, + /// Optional `ALLOW_WRITES` setting. + allow_writes: Option, + /// Optional comment. + comment: Option, + }, + /// ```sql + /// ALTER EXTERNAL VOLUME [IF EXISTS] ... + /// ``` + AlterExternalVolume { + /// External volume name. + name: ObjectName, + /// `IF EXISTS` flag. + if_exists: bool, + /// The alter operation. + operation: AlterExternalVolumeOperation, + }, + /// ```sql + /// DROP EXTERNAL VOLUME [IF EXISTS] + /// ``` + DropExternalVolume { + /// External volume name. + name: ObjectName, + /// `IF EXISTS` flag. + if_exists: bool, + }, + /// ```sql + /// DESC[RIBE] EXTERNAL VOLUME + /// ``` + DescribeExternalVolume { + /// External volume name. + name: ObjectName, + }, + /// ```sql + /// SHOW EXTERNAL VOLUMES [LIKE ''] + /// ``` + ShowExternalVolumes { + /// Optional filter (e.g. `LIKE`). + filter: Option, + }, + /// ```sql /// ASSERT [AS ] /// ``` Assert { @@ -6261,6 +6313,63 @@ impl fmt::Display for Statement { } Ok(()) } + Statement::CreateExternalVolume { + or_replace, + if_not_exists, + name, + storage_locations, + allow_writes, + comment, + } => { + write!( + f, + "CREATE {or_replace}EXTERNAL VOLUME {if_not_exists}{name} STORAGE_LOCATIONS = (", + or_replace = if *or_replace { "OR REPLACE " } else { "" }, + if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" }, + )?; + for (i, loc) in storage_locations.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "({loc})")?; + } + write!(f, ")")?; + if let Some(val) = allow_writes { + write!(f, " ALLOW_WRITES = {}", if *val { "TRUE" } else { "FALSE" })?; + } + if let Some(ref c) = comment { + write!(f, " COMMENT = '{}'", value::escape_single_quote_string(c))?; + } + Ok(()) + } + Statement::AlterExternalVolume { + name, + if_exists, + operation, + } => { + write!( + f, + "ALTER EXTERNAL VOLUME {if_exists}{name} {operation}", + if_exists = if *if_exists { "IF EXISTS " } else { "" }, + ) + } + Statement::DropExternalVolume { name, if_exists } => { + write!( + f, + "DROP EXTERNAL VOLUME {if_exists}{name}", + if_exists = if *if_exists { "IF EXISTS " } else { "" }, + ) + } + Statement::DescribeExternalVolume { name } => { + write!(f, "DESCRIBE EXTERNAL VOLUME {name}") + } + Statement::ShowExternalVolumes { filter } => { + write!(f, "SHOW EXTERNAL VOLUMES")?; + if let Some(ref filter) = filter { + write!(f, " {filter}")?; + } + Ok(()) + } Statement::CopyIntoSnowflake { kind, into, @@ -11021,6 +11130,121 @@ pub struct ShowObjects { pub show_options: ShowStatementOptions, } +/// A storage location within a Snowflake `EXTERNAL VOLUME`. +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub struct ExternalVolumeStorageLocation { + /// The `NAME` of the storage location. + pub name: String, + /// The `STORAGE_PROVIDER` (e.g. `'S3'`). + pub storage_provider: String, + /// The `STORAGE_BASE_URL` (e.g. `'s3://bucket/path/'`). + pub storage_base_url: String, + /// Optional `STORAGE_AWS_ROLE_ARN`. + pub storage_aws_role_arn: Option, + /// Optional `STORAGE_AWS_EXTERNAL_ID`. + pub storage_aws_external_id: Option, + /// Optional `ENCRYPTION` settings. + pub encryption: Option, +} + +impl fmt::Display for ExternalVolumeStorageLocation { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "NAME = '{}' STORAGE_PROVIDER = '{}' STORAGE_BASE_URL = '{}'", + value::escape_single_quote_string(&self.name), + value::escape_single_quote_string(&self.storage_provider), + value::escape_single_quote_string(&self.storage_base_url), + )?; + if let Some(ref arn) = self.storage_aws_role_arn { + write!( + f, + " STORAGE_AWS_ROLE_ARN = '{}'", + value::escape_single_quote_string(arn) + )?; + } + if let Some(ref ext_id) = self.storage_aws_external_id { + write!( + f, + " STORAGE_AWS_EXTERNAL_ID = '{}'", + value::escape_single_quote_string(ext_id) + )?; + } + if let Some(ref enc) = self.encryption { + write!(f, " ENCRYPTION = ({enc})")?; + } + Ok(()) + } +} + +/// Encryption settings for an `EXTERNAL VOLUME` storage location. +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub struct ExternalVolumeEncryption { + /// Encryption type: `'AWS_SSE_S3'`, `'AWS_SSE_KMS'`, or `'NONE'`. + pub kind: String, + /// Optional `KMS_KEY_ID`. + pub kms_key_id: Option, +} + +impl fmt::Display for ExternalVolumeEncryption { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!( + f, + "TYPE = '{}'", + value::escape_single_quote_string(&self.kind) + )?; + if let Some(ref key_id) = self.kms_key_id { + write!( + f, + " KMS_KEY_ID = '{}'", + value::escape_single_quote_string(key_id) + )?; + } + Ok(()) + } +} + +/// Operations for `ALTER EXTERNAL VOLUME`. +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub enum AlterExternalVolumeOperation { + /// `ADD STORAGE_LOCATION = ( ... )` + AddStorageLocation(ExternalVolumeStorageLocation), + /// `SET ALLOW_WRITES = TRUE|FALSE` + SetAllowWrites(bool), + /// `REMOVE STORAGE_LOCATION ''` + RemoveStorageLocation(String), +} + +impl fmt::Display for AlterExternalVolumeOperation { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + AlterExternalVolumeOperation::AddStorageLocation(loc) => { + write!(f, "ADD STORAGE_LOCATION = ({loc})") + } + AlterExternalVolumeOperation::SetAllowWrites(val) => { + write!( + f, + "SET ALLOW_WRITES = {}", + if *val { "TRUE" } else { "FALSE" } + ) + } + AlterExternalVolumeOperation::RemoveStorageLocation(name) => { + write!( + f, + "REMOVE STORAGE_LOCATION '{}'", + value::escape_single_quote_string(name) + ) + } + } + } +} + /// MSSQL's json null clause /// /// ```plaintext diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 6505b209e..896da4ef6 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -522,6 +522,11 @@ impl Spanned for Statement { Statement::Vacuum(..) => Span::empty(), Statement::AlterUser(..) => Span::empty(), Statement::Reset(..) => Span::empty(), + Statement::CreateExternalVolume { .. } => Span::empty(), + Statement::AlterExternalVolume { .. } => Span::empty(), + Statement::DropExternalVolume { .. } => Span::empty(), + Statement::DescribeExternalVolume { .. } => Span::empty(), + Statement::ShowExternalVolumes { .. } => Span::empty(), } } } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index b76cbc55e..fee7613e8 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -27,14 +27,16 @@ use crate::ast::helpers::stmt_data_loading::{ FileStagingCommand, StageLoadSelectItem, StageLoadSelectItemKind, StageParamsObject, }; use crate::ast::{ - AlterTable, AlterTableOperation, AlterTableType, CatalogSyncNamespaceMode, ColumnOption, - ColumnPolicy, ColumnPolicyProperty, ContactEntry, CopyIntoSnowflakeKind, CreateTable, - CreateTableLikeKind, DollarQuotedString, Ident, IdentityParameters, IdentityProperty, - IdentityPropertyFormatKind, IdentityPropertyKind, IdentityPropertyOrder, InitializeKind, - Insert, MultiTableInsertIntoClause, MultiTableInsertType, MultiTableInsertValue, - MultiTableInsertValues, MultiTableInsertWhenClause, ObjectName, ObjectNamePart, - RefreshModeKind, RowAccessPolicy, ShowObjects, SqlOption, Statement, StorageLifecyclePolicy, - StorageSerializationPolicy, TableObject, TagsColumnOption, Value, WrappedCollection, + AlterExternalVolumeOperation, AlterTable, AlterTableOperation, AlterTableType, + CatalogSyncNamespaceMode, ColumnOption, ColumnPolicy, ColumnPolicyProperty, ContactEntry, + CopyIntoSnowflakeKind, CreateTable, CreateTableLikeKind, DollarQuotedString, + ExternalVolumeEncryption, ExternalVolumeStorageLocation, Ident, IdentityParameters, + IdentityProperty, IdentityPropertyFormatKind, IdentityPropertyKind, IdentityPropertyOrder, + InitializeKind, Insert, MultiTableInsertIntoClause, MultiTableInsertType, + MultiTableInsertValue, MultiTableInsertValues, MultiTableInsertWhenClause, ObjectName, + ObjectNamePart, RefreshModeKind, RowAccessPolicy, ShowObjects, SqlOption, Statement, + StorageLifecyclePolicy, StorageSerializationPolicy, TableObject, TagsColumnOption, Value, + WrappedCollection, }; use crate::dialect::{Dialect, Precedence}; use crate::keywords::Keyword; @@ -266,6 +268,11 @@ impl Dialect for SnowflakeDialect { return Some(parse_alter_dynamic_table(parser)); } + if parser.parse_keywords(&[Keyword::ALTER, Keyword::EXTERNAL, Keyword::VOLUME]) { + // ALTER EXTERNAL VOLUME + return Some(parse_alter_external_volume(parser)); + } + if parser.parse_keywords(&[Keyword::ALTER, Keyword::EXTERNAL, Keyword::TABLE]) { // ALTER EXTERNAL TABLE return Some(parse_alter_external_table(parser)); @@ -281,10 +288,33 @@ impl Dialect for SnowflakeDialect { return Some(parse_alter_session(parser, set)); } + if parser.parse_keywords(&[Keyword::DROP, Keyword::EXTERNAL, Keyword::VOLUME]) { + // DROP EXTERNAL VOLUME + return Some(parse_drop_external_volume(parser)); + } + + if parser + .parse_one_of_keywords(&[Keyword::DESC, Keyword::DESCRIBE]) + .is_some() + { + if parser.parse_keywords(&[Keyword::EXTERNAL, Keyword::VOLUME]) { + // DESC[RIBE] EXTERNAL VOLUME + return Some(parse_describe_external_volume(parser)); + } + // not EXTERNAL VOLUME — put back DESC/DESCRIBE + parser.prev_token(); + } + if parser.parse_keyword(Keyword::CREATE) { // possibly CREATE STAGE //[ OR REPLACE ] let or_replace = parser.parse_keywords(&[Keyword::OR, Keyword::REPLACE]); + + // CREATE [OR REPLACE] EXTERNAL VOLUME + if parser.parse_keywords(&[Keyword::EXTERNAL, Keyword::VOLUME]) { + return Some(parse_create_external_volume(or_replace, parser)); + } + // LOCAL | GLOBAL let global = match parser.parse_one_of_keywords(&[Keyword::LOCAL, Keyword::GLOBAL]) { Some(Keyword::LOCAL) => Some(false), @@ -363,6 +393,9 @@ impl Dialect for SnowflakeDialect { } if parser.parse_keyword(Keyword::SHOW) { + if parser.parse_keywords(&[Keyword::EXTERNAL, Keyword::VOLUMES]) { + return Some(parse_show_external_volumes(parser)); + } let terse = parser.parse_keyword(Keyword::TERSE); if parser.parse_keyword(Keyword::OBJECTS) { return Some(parse_show_objects(terse, parser)); @@ -1983,3 +2016,204 @@ fn parse_multi_table_insert_when_clauses( Ok((when_clauses, else_clause)) } + +/// Parse `CREATE [OR REPLACE] EXTERNAL VOLUME [IF NOT EXISTS] ...` +fn parse_create_external_volume( + or_replace: bool, + parser: &mut Parser, +) -> Result { + let if_not_exists = parser.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); + let name = parser.parse_object_name(false)?; + + parser.expect_keyword(Keyword::STORAGE_LOCATIONS)?; + parser.expect_token(&Token::Eq)?; + parser.expect_token(&Token::LParen)?; + + let mut storage_locations = vec![]; + loop { + parser.expect_token(&Token::LParen)?; + storage_locations.push(parse_external_volume_storage_location(parser)?); + parser.expect_token(&Token::RParen)?; + if !parser.consume_token(&Token::Comma) { + break; + } + } + parser.expect_token(&Token::RParen)?; + + let mut allow_writes = None; + let mut comment = None; + + if parser.parse_keyword(Keyword::ALLOW_WRITES) { + parser.expect_token(&Token::Eq)?; + allow_writes = Some(parser.parse_boolean_string()?); + } + + if parser.parse_keyword(Keyword::COMMENT) { + parser.expect_token(&Token::Eq)?; + comment = Some(parser.parse_comment_value()?); + } + + Ok(Statement::CreateExternalVolume { + or_replace, + if_not_exists, + name, + storage_locations, + allow_writes, + comment, + }) +} + +/// Parse a single storage location: `NAME = '...' STORAGE_PROVIDER = '...' ...` +/// +/// `NAME` and `STORAGE_PROVIDER` must appear first (in that order). After +/// those two, `STORAGE_BASE_URL` (required) and the optional fields +/// (`STORAGE_AWS_ROLE_ARN`, `STORAGE_AWS_EXTERNAL_ID`, `ENCRYPTION`) may +/// appear in any order, separated by commas or whitespace. Real Snowflake +/// accepts the flexible ordering — mirror that here so fixtures like +/// `NAME = '…' STORAGE_PROVIDER = '…' STORAGE_AWS_ROLE_ARN = '…' +/// STORAGE_BASE_URL = '…'` parse correctly. +fn parse_external_volume_storage_location( + parser: &mut Parser, +) -> Result { + parser.expect_keyword(Keyword::NAME)?; + parser.expect_token(&Token::Eq)?; + let name = parser.parse_literal_string()?; + + // separators may be commas or whitespace — consume optional comma + let _ = parser.consume_token(&Token::Comma); + + parser.expect_keyword(Keyword::STORAGE_PROVIDER)?; + parser.expect_token(&Token::Eq)?; + let storage_provider = parser.parse_literal_string()?; + + let mut storage_base_url: Option = None; + let mut storage_aws_role_arn = None; + let mut storage_aws_external_id = None; + let mut encryption = None; + + // Parse the remaining fields (STORAGE_BASE_URL + optionals) in any order + // (comma-or-whitespace separated). Duplicates are rejected. + loop { + let _ = parser.consume_token(&Token::Comma); + if parser.parse_keyword(Keyword::STORAGE_BASE_URL) { + if storage_base_url.is_some() { + return Err(ParserError::ParserError( + "duplicate STORAGE_BASE_URL in STORAGE_LOCATION".to_string(), + )); + } + parser.expect_token(&Token::Eq)?; + storage_base_url = Some(parser.parse_literal_string()?); + } else if parser.parse_keyword(Keyword::STORAGE_AWS_ROLE_ARN) { + if storage_aws_role_arn.is_some() { + return Err(ParserError::ParserError( + "duplicate STORAGE_AWS_ROLE_ARN in STORAGE_LOCATION".to_string(), + )); + } + parser.expect_token(&Token::Eq)?; + storage_aws_role_arn = Some(parser.parse_literal_string()?); + } else if parser.parse_keyword(Keyword::STORAGE_AWS_EXTERNAL_ID) { + if storage_aws_external_id.is_some() { + return Err(ParserError::ParserError( + "duplicate STORAGE_AWS_EXTERNAL_ID in STORAGE_LOCATION".to_string(), + )); + } + parser.expect_token(&Token::Eq)?; + storage_aws_external_id = Some(parser.parse_literal_string()?); + } else if parser.parse_keyword(Keyword::ENCRYPTION) { + if encryption.is_some() { + return Err(ParserError::ParserError( + "duplicate ENCRYPTION in STORAGE_LOCATION".to_string(), + )); + } + parser.expect_token(&Token::Eq)?; + parser.expect_token(&Token::LParen)?; + encryption = Some(parse_external_volume_encryption(parser)?); + parser.expect_token(&Token::RParen)?; + } else { + break; + } + } + + let storage_base_url = storage_base_url.ok_or_else(|| { + ParserError::ParserError("STORAGE_BASE_URL is required in STORAGE_LOCATION".to_string()) + })?; + + Ok(ExternalVolumeStorageLocation { + name, + storage_provider, + storage_base_url, + storage_aws_role_arn, + storage_aws_external_id, + encryption, + }) +} + +/// Parse encryption options: `TYPE = '...' [KMS_KEY_ID = '...']` +fn parse_external_volume_encryption( + parser: &mut Parser, +) -> Result { + parser.expect_keyword(Keyword::TYPE)?; + parser.expect_token(&Token::Eq)?; + let kind = parser.parse_literal_string()?; + + let mut kms_key_id = None; + if parser.parse_keyword(Keyword::KMS_KEY_ID) { + parser.expect_token(&Token::Eq)?; + kms_key_id = Some(parser.parse_literal_string()?); + } + + Ok(ExternalVolumeEncryption { kind, kms_key_id }) +} + +/// Parse `ALTER EXTERNAL VOLUME [IF EXISTS] ...` +fn parse_alter_external_volume(parser: &mut Parser) -> Result { + let if_exists = parser.parse_keywords(&[Keyword::IF, Keyword::EXISTS]); + let name = parser.parse_object_name(false)?; + + let operation = if parser.parse_keyword(Keyword::ADD) { + parser.expect_keyword(Keyword::STORAGE_LOCATION)?; + parser.expect_token(&Token::Eq)?; + parser.expect_token(&Token::LParen)?; + let loc = parse_external_volume_storage_location(parser)?; + parser.expect_token(&Token::RParen)?; + AlterExternalVolumeOperation::AddStorageLocation(loc) + } else if parser.parse_keyword(Keyword::SET) { + parser.expect_keyword(Keyword::ALLOW_WRITES)?; + parser.expect_token(&Token::Eq)?; + AlterExternalVolumeOperation::SetAllowWrites(parser.parse_boolean_string()?) + } else if parser.parse_keyword(Keyword::REMOVE) { + parser.expect_keyword(Keyword::STORAGE_LOCATION)?; + let loc_name = parser.parse_literal_string()?; + AlterExternalVolumeOperation::RemoveStorageLocation(loc_name) + } else { + return parser.expected( + "ADD, SET, or REMOVE after ALTER EXTERNAL VOLUME ", + parser.peek_token(), + ); + }; + + Ok(Statement::AlterExternalVolume { + name, + if_exists, + operation, + }) +} + +/// Parse `DROP EXTERNAL VOLUME [IF EXISTS] ` +fn parse_drop_external_volume(parser: &mut Parser) -> Result { + let if_exists = parser.parse_keywords(&[Keyword::IF, Keyword::EXISTS]); + let name = parser.parse_object_name(false)?; + Ok(Statement::DropExternalVolume { name, if_exists }) +} + +/// Parse `DESC[RIBE] EXTERNAL VOLUME ` +fn parse_describe_external_volume(parser: &mut Parser) -> Result { + let name = parser.parse_object_name(false)?; + Ok(Statement::DescribeExternalVolume { name }) +} + +/// Parse `SHOW EXTERNAL VOLUMES [LIKE '']` +fn parse_show_external_volumes(parser: &mut Parser) -> Result { + let filter = parser.parse_show_statement_filter()?; + Ok(Statement::ShowExternalVolumes { filter }) +} diff --git a/src/keywords.rs b/src/keywords.rs index c2d0a47ec..d3163c6b2 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -112,6 +112,7 @@ define_keywords!( ALL, ALLOCATE, ALLOWOVERWRITE, + ALLOW_WRITES, ALTER, ALWAYS, ANALYZE, @@ -569,6 +570,7 @@ define_keywords!( KEYS, KEY_BLOCK_SIZE, KILL, + KMS_KEY_ID, LAG, LAMBDA, LANGUAGE, @@ -1004,7 +1006,13 @@ define_keywords!( STDOUT, STEP, STORAGE, + STORAGE_AWS_EXTERNAL_ID, + STORAGE_AWS_ROLE_ARN, + STORAGE_BASE_URL, STORAGE_INTEGRATION, + STORAGE_LOCATION, + STORAGE_LOCATIONS, + STORAGE_PROVIDER, STORAGE_SERIALIZATION_POLICY, STORED, STRAIGHT_JOIN, @@ -1161,6 +1169,7 @@ define_keywords!( VIRTUAL, VOLATILE, VOLUME, + VOLUMES, WAITFOR, WAREHOUSE, WAREHOUSES, diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index 6bebd5863..c3d05275a 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -4914,3 +4914,595 @@ fn test_select_dollar_column_from_stage() { // With table function args, without alias snowflake().verified_stmt("SELECT $1, $2 FROM @mystage1(file_format => 'myformat')"); } + +#[test] +fn test_create_external_volume_basic() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/path/'))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + or_replace, + if_not_exists, + name, + storage_locations, + allow_writes, + comment, + } => { + assert!(!or_replace); + assert!(!if_not_exists); + assert_eq!("my_vol", name.to_string()); + assert_eq!(1, storage_locations.len()); + assert_eq!("loc1", storage_locations[0].name); + assert_eq!("S3", storage_locations[0].storage_provider); + assert_eq!("s3://bucket/path/", storage_locations[0].storage_base_url); + assert!(storage_locations[0].storage_aws_role_arn.is_none()); + assert!(storage_locations[0].encryption.is_none()); + assert!(allow_writes.is_none()); + assert!(comment.is_none()); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_or_replace() { + let sql = concat!( + "CREATE OR REPLACE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/'))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { or_replace, .. } => { + assert!(or_replace); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_if_not_exists() { + let sql = concat!( + "CREATE EXTERNAL VOLUME IF NOT EXISTS my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/'))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { if_not_exists, .. } => { + assert!(if_not_exists); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_multi_location() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket1/'), ", + "(NAME = 'loc2' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket2/' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/myrole'))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + storage_locations, .. + } => { + assert_eq!(2, storage_locations.len()); + assert_eq!("loc1", storage_locations[0].name); + assert_eq!("loc2", storage_locations[1].name); + assert_eq!( + Some("arn:aws:iam::role/myrole".to_string()), + storage_locations[1].storage_aws_role_arn + ); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_with_encryption_sse_s3() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", + "ENCRYPTION = (TYPE = 'AWS_SSE_S3')))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + storage_locations, .. + } => { + let enc = storage_locations[0].encryption.as_ref().unwrap(); + assert_eq!("AWS_SSE_S3", enc.kind); + assert!(enc.kms_key_id.is_none()); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_with_encryption_kms() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", + "ENCRYPTION = (TYPE = 'AWS_SSE_KMS' KMS_KEY_ID = 'my-key-id')))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + storage_locations, .. + } => { + let enc = storage_locations[0].encryption.as_ref().unwrap(); + assert_eq!("AWS_SSE_KMS", enc.kind); + assert_eq!(Some("my-key-id".to_string()), enc.kms_key_id); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_with_encryption_none() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", + "ENCRYPTION = (TYPE = 'NONE')))", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + storage_locations, .. + } => { + let enc = storage_locations[0].encryption.as_ref().unwrap(); + assert_eq!("NONE", enc.kind); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_full() { + let sql = concat!( + "CREATE OR REPLACE EXTERNAL VOLUME IF NOT EXISTS my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r' ", + "STORAGE_AWS_EXTERNAL_ID = 'ext-id' ", + "ENCRYPTION = (TYPE = 'AWS_SSE_KMS' KMS_KEY_ID = 'key'))) ", + "ALLOW_WRITES = TRUE ", + "COMMENT = 'my comment'", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + or_replace, + if_not_exists, + storage_locations, + allow_writes, + comment, + .. + } => { + assert!(or_replace); + assert!(if_not_exists); + assert_eq!(1, storage_locations.len()); + assert_eq!( + Some("ext-id".to_string()), + storage_locations[0].storage_aws_external_id + ); + assert_eq!(Some(true), allow_writes); + assert_eq!(Some("my comment".to_string()), comment); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_allow_writes_false() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/')) ", + "ALLOW_WRITES = FALSE", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { allow_writes, .. } => { + assert_eq!(Some(false), allow_writes); + } + _ => unreachable!(), + } +} + +#[test] +fn test_alter_external_volume_add_storage_location() { + let sql = concat!( + "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ", + "(NAME = 'loc2' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket2/')", + ); + match snowflake().verified_stmt(sql) { + Statement::AlterExternalVolume { + name, + if_exists, + operation, + } => { + assert_eq!("my_vol", name.to_string()); + assert!(!if_exists); + match operation { + AlterExternalVolumeOperation::AddStorageLocation(loc) => { + assert_eq!("loc2", loc.name); + assert_eq!("S3", loc.storage_provider); + } + _ => unreachable!(), + } + } + _ => unreachable!(), + } +} + +#[test] +fn test_alter_external_volume_set_allow_writes() { + let sql = "ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = TRUE"; + match snowflake().verified_stmt(sql) { + Statement::AlterExternalVolume { operation, .. } => { + assert_eq!( + AlterExternalVolumeOperation::SetAllowWrites(true), + operation + ); + } + _ => unreachable!(), + } + + let sql = "ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = FALSE"; + match snowflake().verified_stmt(sql) { + Statement::AlterExternalVolume { operation, .. } => { + assert_eq!( + AlterExternalVolumeOperation::SetAllowWrites(false), + operation + ); + } + _ => unreachable!(), + } +} + +#[test] +fn test_alter_external_volume_if_exists() { + let sql = "ALTER EXTERNAL VOLUME IF EXISTS my_vol SET ALLOW_WRITES = TRUE"; + match snowflake().verified_stmt(sql) { + Statement::AlterExternalVolume { if_exists, .. } => { + assert!(if_exists); + } + _ => unreachable!(), + } +} + +#[test] +fn test_alter_external_volume_remove_storage_location() { + let sql = "ALTER EXTERNAL VOLUME my_vol REMOVE STORAGE_LOCATION 'loc1'"; + match snowflake().verified_stmt(sql) { + Statement::AlterExternalVolume { operation, .. } => { + assert_eq!( + AlterExternalVolumeOperation::RemoveStorageLocation("loc1".to_string()), + operation + ); + } + _ => unreachable!(), + } +} + +#[test] +fn test_drop_external_volume() { + let sql = "DROP EXTERNAL VOLUME my_vol"; + match snowflake().verified_stmt(sql) { + Statement::DropExternalVolume { name, if_exists } => { + assert_eq!("my_vol", name.to_string()); + assert!(!if_exists); + } + _ => unreachable!(), + } +} + +#[test] +fn test_drop_external_volume_if_exists() { + let sql = "DROP EXTERNAL VOLUME IF EXISTS my_vol"; + match snowflake().verified_stmt(sql) { + Statement::DropExternalVolume { if_exists, .. } => { + assert!(if_exists); + } + _ => unreachable!(), + } +} + +#[test] +fn test_describe_external_volume() { + let sql = "DESCRIBE EXTERNAL VOLUME my_vol"; + match snowflake().verified_stmt(sql) { + Statement::DescribeExternalVolume { name } => { + assert_eq!("my_vol", name.to_string()); + } + _ => unreachable!(), + } +} + +#[test] +fn test_show_external_volumes() { + let sql = "SHOW EXTERNAL VOLUMES"; + match snowflake().verified_stmt(sql) { + Statement::ShowExternalVolumes { filter } => { + assert!(filter.is_none()); + } + _ => unreachable!(), + } +} + +#[test] +fn test_show_external_volumes_like() { + let sql = "SHOW EXTERNAL VOLUMES LIKE 'my_%'"; + match snowflake().verified_stmt(sql) { + Statement::ShowExternalVolumes { filter } => { + assert!(filter.is_some()); + match filter.unwrap() { + ShowStatementFilter::Like(pattern) => { + assert_eq!("my_%", pattern); + } + _ => unreachable!(), + } + } + _ => unreachable!(), + } +} + +#[test] +fn test_desc_external_volume() { + let sql = "DESC EXTERNAL VOLUME my_vol"; + let canonical = "DESCRIBE EXTERNAL VOLUME my_vol"; + match snowflake().one_statement_parses_to(sql, canonical) { + Statement::DescribeExternalVolume { name } => { + assert_eq!("my_vol", name.to_string()); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_comma_separated_fields() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1', STORAGE_PROVIDER = 'S3', STORAGE_BASE_URL = 's3://bucket/', ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", + ); + let canonical = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", + ); + match snowflake().one_statement_parses_to(sql, canonical) { + Statement::CreateExternalVolume { + storage_locations, .. + } => { + assert_eq!(1, storage_locations.len()); + assert_eq!("loc1", storage_locations[0].name); + assert_eq!("S3", storage_locations[0].storage_provider); + assert_eq!("s3://bucket/", storage_locations[0].storage_base_url); + assert_eq!( + Some("arn:aws:iam::role/r".to_string()), + storage_locations[0].storage_aws_role_arn + ); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_flexible_field_ordering() { + // Real Snowflake accepts STORAGE_BASE_URL anywhere after STORAGE_PROVIDER. + // Here STORAGE_AWS_ROLE_ARN appears before STORAGE_BASE_URL. + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r' ", + "STORAGE_BASE_URL = 's3://bucket/'))", + ); + let canonical = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", + ); + match snowflake().one_statement_parses_to(sql, canonical) { + Statement::CreateExternalVolume { + storage_locations, .. + } => { + assert_eq!(1, storage_locations.len()); + assert_eq!("loc1", storage_locations[0].name); + assert_eq!("S3", storage_locations[0].storage_provider); + assert_eq!("s3://bucket/", storage_locations[0].storage_base_url); + assert_eq!( + Some("arn:aws:iam::role/r".to_string()), + storage_locations[0].storage_aws_role_arn + ); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_missing_storage_base_url() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", + ); + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject STORAGE_LOCATION without STORAGE_BASE_URL"); + assert!( + err.to_string().contains("STORAGE_BASE_URL"), + "unexpected error: {err}" + ); +} + +#[test] +fn test_create_external_volume_duplicate_storage_base_url() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' ", + "STORAGE_BASE_URL = 's3://bucket/' ", + "STORAGE_BASE_URL = 's3://other/'))", + ); + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject duplicate STORAGE_BASE_URL"); + assert!( + err.to_string().contains("duplicate STORAGE_BASE_URL"), + "unexpected error: {err}" + ); +} + +#[test] +fn test_create_external_volume_duplicate_optional_fields() { + // Each optional field is rejected when repeated within a STORAGE_LOCATION. + let cases = [ + ( + "STORAGE_AWS_ROLE_ARN = 'a' STORAGE_AWS_ROLE_ARN = 'b'", + "duplicate STORAGE_AWS_ROLE_ARN", + ), + ( + "STORAGE_AWS_EXTERNAL_ID = 'a' STORAGE_AWS_EXTERNAL_ID = 'b'", + "duplicate STORAGE_AWS_EXTERNAL_ID", + ), + ( + "ENCRYPTION = (TYPE = 'NONE') ENCRYPTION = (TYPE = 'NONE')", + "duplicate ENCRYPTION", + ), + ]; + for (fields, expected) in cases { + let sql = format!( + "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = (\ + (NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' {fields}))" + ); + let err = snowflake() + .parse_sql_statements(&sql) + .expect_err("parser must reject duplicate field"); + assert!( + err.to_string().contains(expected), + "expected `{expected}`, got: {err}" + ); + } +} + +#[test] +fn test_create_external_volume_empty_storage_locations() { + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = ()"; + snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject empty STORAGE_LOCATIONS"); +} + +#[test] +fn test_create_external_volume_allow_writes_non_boolean() { + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/')) ", + "ALLOW_WRITES = 1", + ); + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject non-boolean ALLOW_WRITES"); + assert!( + err.to_string().contains("TRUE or FALSE"), + "unexpected error: {err}" + ); +} + +#[test] +fn test_create_external_volume_escaped_single_quotes() { + // Single quotes inside string values round-trip through escaping. + let sql = concat!( + "CREATE EXTERNAL VOLUME my_vol ", + "STORAGE_LOCATIONS = (", + "(NAME = 'lo''c1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/')) ", + "COMMENT = 'it''s mine'", + ); + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + storage_locations, + comment, + .. + } => { + assert_eq!("lo'c1", storage_locations[0].name); + assert_eq!(Some("it's mine".to_string()), comment); + } + _ => unreachable!(), + } +} + +#[test] +fn test_alter_external_volume_add_storage_location_full() { + // The ADD path reuses the storage-location parser, so exercise the + // optional fields (external id + encryption) through it too. + let sql = concat!( + "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ", + "(NAME = 'loc2' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket2/' ", + "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r' ", + "STORAGE_AWS_EXTERNAL_ID = 'ext-id' ", + "ENCRYPTION = (TYPE = 'AWS_SSE_KMS' KMS_KEY_ID = 'key'))", + ); + match snowflake().verified_stmt(sql) { + Statement::AlterExternalVolume { operation, .. } => match operation { + AlterExternalVolumeOperation::AddStorageLocation(loc) => { + assert_eq!("loc2", loc.name); + assert_eq!(Some("ext-id".to_string()), loc.storage_aws_external_id); + let enc = loc.encryption.as_ref().unwrap(); + assert_eq!("AWS_SSE_KMS", enc.kind); + assert_eq!(Some("key".to_string()), enc.kms_key_id); + } + _ => unreachable!(), + }, + _ => unreachable!(), + } +} + +#[test] +fn test_alter_external_volume_add_missing_storage_base_url() { + // The ADD path enforces the same required-field rule as CREATE. + let sql = concat!( + "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ", + "(NAME = 'loc2' STORAGE_PROVIDER = 'S3')", + ); + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject ADD STORAGE_LOCATION without STORAGE_BASE_URL"); + assert!( + err.to_string().contains("STORAGE_BASE_URL"), + "unexpected error: {err}" + ); +} + +#[test] +fn test_alter_external_volume_allow_writes_non_boolean() { + let sql = "ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = 1"; + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject non-boolean ALLOW_WRITES"); + assert!( + err.to_string().contains("TRUE or FALSE"), + "unexpected error: {err}" + ); +} + +#[test] +fn test_alter_external_volume_missing_operation() { + let sql = "ALTER EXTERNAL VOLUME my_vol"; + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject ALTER EXTERNAL VOLUME without an operation"); + assert!( + err.to_string().contains("ADD, SET, or REMOVE"), + "unexpected error: {err}" + ); +} From dcbada661389f59f258df00fcf3bae45272a8a47 Mon Sep 17 00:00:00 2001 From: sabir Date: Fri, 3 Jul 2026 10:32:35 +0200 Subject: [PATCH 2/4] Snowflake EXTERNAL VOLUME: reuse KeyValueOptions and generic DROP Address review feedback by building on existing infrastructure instead of bespoke AST: - Parse storage locations (and the nested ENCRYPTION clause) via Parser::parse_key_value_options, the same path CREATE STAGE uses. Storage locations are now Vec; the ExternalVolumeStorageLocation and ExternalVolumeEncryption structs and their hand-written Display and parser code are removed. Parsing is syntax-only, so field order is preserved and semantic checks (required STORAGE_BASE_URL, duplicate fields) are left to the consumer. - Route DROP EXTERNAL VOLUME through the generic Statement::Drop machinery via a new ObjectType::ExternalVolume, matching how STAGE/STREAM are handled, and drop the dedicated DropExternalVolume statement variant. - Use expect_keyword_is instead of expect_keyword to match the file's convention, and remove the now-unused option-name keywords. Co-Authored-By: Claude Opus 4.8 --- src/ast/mod.rs | 104 +----- src/ast/spans.rs | 1 - src/dialect/snowflake.rs | 151 ++------- src/keywords.rs | 5 - src/parser/mod.rs | 4 +- tests/sqlparser_snowflake.rs | 608 ++++++++++++++--------------------- 6 files changed, 265 insertions(+), 608 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 540c12488..ea256d6f1 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -4546,8 +4546,9 @@ pub enum Statement { if_not_exists: bool, /// External volume name. name: ObjectName, - /// Storage locations. - storage_locations: Vec, + /// Storage locations, each a parenthesized list of key-value options + /// (e.g. `(NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/')`). + storage_locations: Vec, /// Optional `ALLOW_WRITES` setting. allow_writes: Option, /// Optional comment. @@ -4565,15 +4566,6 @@ pub enum Statement { operation: AlterExternalVolumeOperation, }, /// ```sql - /// DROP EXTERNAL VOLUME [IF EXISTS] - /// ``` - DropExternalVolume { - /// External volume name. - name: ObjectName, - /// `IF EXISTS` flag. - if_exists: bool, - }, - /// ```sql /// DESC[RIBE] EXTERNAL VOLUME /// ``` DescribeExternalVolume { @@ -6353,13 +6345,6 @@ impl fmt::Display for Statement { if_exists = if *if_exists { "IF EXISTS " } else { "" }, ) } - Statement::DropExternalVolume { name, if_exists } => { - write!( - f, - "DROP EXTERNAL VOLUME {if_exists}{name}", - if_exists = if *if_exists { "IF EXISTS " } else { "" }, - ) - } Statement::DescribeExternalVolume { name } => { write!(f, "DESCRIBE EXTERNAL VOLUME {name}") } @@ -8688,6 +8673,8 @@ pub enum ObjectType { User, /// A stream. Stream, + /// A Snowflake external volume. + ExternalVolume, } impl fmt::Display for ObjectType { @@ -8706,6 +8693,7 @@ impl fmt::Display for ObjectType { ObjectType::Type => "TYPE", ObjectType::User => "USER", ObjectType::Stream => "STREAM", + ObjectType::ExternalVolume => "EXTERNAL VOLUME", }) } } @@ -11130,91 +11118,13 @@ pub struct ShowObjects { pub show_options: ShowStatementOptions, } -/// A storage location within a Snowflake `EXTERNAL VOLUME`. -#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub struct ExternalVolumeStorageLocation { - /// The `NAME` of the storage location. - pub name: String, - /// The `STORAGE_PROVIDER` (e.g. `'S3'`). - pub storage_provider: String, - /// The `STORAGE_BASE_URL` (e.g. `'s3://bucket/path/'`). - pub storage_base_url: String, - /// Optional `STORAGE_AWS_ROLE_ARN`. - pub storage_aws_role_arn: Option, - /// Optional `STORAGE_AWS_EXTERNAL_ID`. - pub storage_aws_external_id: Option, - /// Optional `ENCRYPTION` settings. - pub encryption: Option, -} - -impl fmt::Display for ExternalVolumeStorageLocation { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "NAME = '{}' STORAGE_PROVIDER = '{}' STORAGE_BASE_URL = '{}'", - value::escape_single_quote_string(&self.name), - value::escape_single_quote_string(&self.storage_provider), - value::escape_single_quote_string(&self.storage_base_url), - )?; - if let Some(ref arn) = self.storage_aws_role_arn { - write!( - f, - " STORAGE_AWS_ROLE_ARN = '{}'", - value::escape_single_quote_string(arn) - )?; - } - if let Some(ref ext_id) = self.storage_aws_external_id { - write!( - f, - " STORAGE_AWS_EXTERNAL_ID = '{}'", - value::escape_single_quote_string(ext_id) - )?; - } - if let Some(ref enc) = self.encryption { - write!(f, " ENCRYPTION = ({enc})")?; - } - Ok(()) - } -} - -/// Encryption settings for an `EXTERNAL VOLUME` storage location. -#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] -#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] -pub struct ExternalVolumeEncryption { - /// Encryption type: `'AWS_SSE_S3'`, `'AWS_SSE_KMS'`, or `'NONE'`. - pub kind: String, - /// Optional `KMS_KEY_ID`. - pub kms_key_id: Option, -} - -impl fmt::Display for ExternalVolumeEncryption { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "TYPE = '{}'", - value::escape_single_quote_string(&self.kind) - )?; - if let Some(ref key_id) = self.kms_key_id { - write!( - f, - " KMS_KEY_ID = '{}'", - value::escape_single_quote_string(key_id) - )?; - } - Ok(()) - } -} - /// Operations for `ALTER EXTERNAL VOLUME`. #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] pub enum AlterExternalVolumeOperation { /// `ADD STORAGE_LOCATION = ( ... )` - AddStorageLocation(ExternalVolumeStorageLocation), + AddStorageLocation(KeyValueOptions), /// `SET ALLOW_WRITES = TRUE|FALSE` SetAllowWrites(bool), /// `REMOVE STORAGE_LOCATION ''` diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 896da4ef6..f40a93027 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -524,7 +524,6 @@ impl Spanned for Statement { Statement::Reset(..) => Span::empty(), Statement::CreateExternalVolume { .. } => Span::empty(), Statement::AlterExternalVolume { .. } => Span::empty(), - Statement::DropExternalVolume { .. } => Span::empty(), Statement::DescribeExternalVolume { .. } => Span::empty(), Statement::ShowExternalVolumes { .. } => Span::empty(), } diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index fee7613e8..157541119 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -29,14 +29,13 @@ use crate::ast::helpers::stmt_data_loading::{ use crate::ast::{ AlterExternalVolumeOperation, AlterTable, AlterTableOperation, AlterTableType, CatalogSyncNamespaceMode, ColumnOption, ColumnPolicy, ColumnPolicyProperty, ContactEntry, - CopyIntoSnowflakeKind, CreateTable, CreateTableLikeKind, DollarQuotedString, - ExternalVolumeEncryption, ExternalVolumeStorageLocation, Ident, IdentityParameters, - IdentityProperty, IdentityPropertyFormatKind, IdentityPropertyKind, IdentityPropertyOrder, - InitializeKind, Insert, MultiTableInsertIntoClause, MultiTableInsertType, - MultiTableInsertValue, MultiTableInsertValues, MultiTableInsertWhenClause, ObjectName, - ObjectNamePart, RefreshModeKind, RowAccessPolicy, ShowObjects, SqlOption, Statement, - StorageLifecyclePolicy, StorageSerializationPolicy, TableObject, TagsColumnOption, Value, - WrappedCollection, + CopyIntoSnowflakeKind, CreateTable, CreateTableLikeKind, DollarQuotedString, Ident, + IdentityParameters, IdentityProperty, IdentityPropertyFormatKind, IdentityPropertyKind, + IdentityPropertyOrder, InitializeKind, Insert, MultiTableInsertIntoClause, + MultiTableInsertType, MultiTableInsertValue, MultiTableInsertValues, + MultiTableInsertWhenClause, ObjectName, ObjectNamePart, RefreshModeKind, RowAccessPolicy, + ShowObjects, SqlOption, Statement, StorageLifecyclePolicy, StorageSerializationPolicy, + TableObject, TagsColumnOption, Value, WrappedCollection, }; use crate::dialect::{Dialect, Precedence}; use crate::keywords::Keyword; @@ -288,11 +287,6 @@ impl Dialect for SnowflakeDialect { return Some(parse_alter_session(parser, set)); } - if parser.parse_keywords(&[Keyword::DROP, Keyword::EXTERNAL, Keyword::VOLUME]) { - // DROP EXTERNAL VOLUME - return Some(parse_drop_external_volume(parser)); - } - if parser .parse_one_of_keywords(&[Keyword::DESC, Keyword::DESCRIBE]) .is_some() @@ -2018,6 +2012,11 @@ fn parse_multi_table_insert_when_clauses( } /// Parse `CREATE [OR REPLACE] EXTERNAL VOLUME [IF NOT EXISTS] ...` +/// +/// Each storage location is a parenthesized `KEY = value` option list; the +/// options (and the `ENCRYPTION = (...)` sub-list) are parsed generically via +/// [`Parser::parse_key_value_options`], so field order and the exact option +/// set are left to the caller rather than validated here. fn parse_create_external_volume( or_replace: bool, parser: &mut Parser, @@ -2025,15 +2024,13 @@ fn parse_create_external_volume( let if_not_exists = parser.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); let name = parser.parse_object_name(false)?; - parser.expect_keyword(Keyword::STORAGE_LOCATIONS)?; + parser.expect_keyword_is(Keyword::STORAGE_LOCATIONS)?; parser.expect_token(&Token::Eq)?; parser.expect_token(&Token::LParen)?; let mut storage_locations = vec![]; loop { - parser.expect_token(&Token::LParen)?; - storage_locations.push(parse_external_volume_storage_location(parser)?); - parser.expect_token(&Token::RParen)?; + storage_locations.push(parser.parse_key_value_options(true, &[])?); if !parser.consume_token(&Token::Comma) { break; } @@ -2063,126 +2060,21 @@ fn parse_create_external_volume( }) } -/// Parse a single storage location: `NAME = '...' STORAGE_PROVIDER = '...' ...` -/// -/// `NAME` and `STORAGE_PROVIDER` must appear first (in that order). After -/// those two, `STORAGE_BASE_URL` (required) and the optional fields -/// (`STORAGE_AWS_ROLE_ARN`, `STORAGE_AWS_EXTERNAL_ID`, `ENCRYPTION`) may -/// appear in any order, separated by commas or whitespace. Real Snowflake -/// accepts the flexible ordering — mirror that here so fixtures like -/// `NAME = '…' STORAGE_PROVIDER = '…' STORAGE_AWS_ROLE_ARN = '…' -/// STORAGE_BASE_URL = '…'` parse correctly. -fn parse_external_volume_storage_location( - parser: &mut Parser, -) -> Result { - parser.expect_keyword(Keyword::NAME)?; - parser.expect_token(&Token::Eq)?; - let name = parser.parse_literal_string()?; - - // separators may be commas or whitespace — consume optional comma - let _ = parser.consume_token(&Token::Comma); - - parser.expect_keyword(Keyword::STORAGE_PROVIDER)?; - parser.expect_token(&Token::Eq)?; - let storage_provider = parser.parse_literal_string()?; - - let mut storage_base_url: Option = None; - let mut storage_aws_role_arn = None; - let mut storage_aws_external_id = None; - let mut encryption = None; - - // Parse the remaining fields (STORAGE_BASE_URL + optionals) in any order - // (comma-or-whitespace separated). Duplicates are rejected. - loop { - let _ = parser.consume_token(&Token::Comma); - if parser.parse_keyword(Keyword::STORAGE_BASE_URL) { - if storage_base_url.is_some() { - return Err(ParserError::ParserError( - "duplicate STORAGE_BASE_URL in STORAGE_LOCATION".to_string(), - )); - } - parser.expect_token(&Token::Eq)?; - storage_base_url = Some(parser.parse_literal_string()?); - } else if parser.parse_keyword(Keyword::STORAGE_AWS_ROLE_ARN) { - if storage_aws_role_arn.is_some() { - return Err(ParserError::ParserError( - "duplicate STORAGE_AWS_ROLE_ARN in STORAGE_LOCATION".to_string(), - )); - } - parser.expect_token(&Token::Eq)?; - storage_aws_role_arn = Some(parser.parse_literal_string()?); - } else if parser.parse_keyword(Keyword::STORAGE_AWS_EXTERNAL_ID) { - if storage_aws_external_id.is_some() { - return Err(ParserError::ParserError( - "duplicate STORAGE_AWS_EXTERNAL_ID in STORAGE_LOCATION".to_string(), - )); - } - parser.expect_token(&Token::Eq)?; - storage_aws_external_id = Some(parser.parse_literal_string()?); - } else if parser.parse_keyword(Keyword::ENCRYPTION) { - if encryption.is_some() { - return Err(ParserError::ParserError( - "duplicate ENCRYPTION in STORAGE_LOCATION".to_string(), - )); - } - parser.expect_token(&Token::Eq)?; - parser.expect_token(&Token::LParen)?; - encryption = Some(parse_external_volume_encryption(parser)?); - parser.expect_token(&Token::RParen)?; - } else { - break; - } - } - - let storage_base_url = storage_base_url.ok_or_else(|| { - ParserError::ParserError("STORAGE_BASE_URL is required in STORAGE_LOCATION".to_string()) - })?; - - Ok(ExternalVolumeStorageLocation { - name, - storage_provider, - storage_base_url, - storage_aws_role_arn, - storage_aws_external_id, - encryption, - }) -} - -/// Parse encryption options: `TYPE = '...' [KMS_KEY_ID = '...']` -fn parse_external_volume_encryption( - parser: &mut Parser, -) -> Result { - parser.expect_keyword(Keyword::TYPE)?; - parser.expect_token(&Token::Eq)?; - let kind = parser.parse_literal_string()?; - - let mut kms_key_id = None; - if parser.parse_keyword(Keyword::KMS_KEY_ID) { - parser.expect_token(&Token::Eq)?; - kms_key_id = Some(parser.parse_literal_string()?); - } - - Ok(ExternalVolumeEncryption { kind, kms_key_id }) -} - /// Parse `ALTER EXTERNAL VOLUME [IF EXISTS] ...` fn parse_alter_external_volume(parser: &mut Parser) -> Result { let if_exists = parser.parse_keywords(&[Keyword::IF, Keyword::EXISTS]); let name = parser.parse_object_name(false)?; let operation = if parser.parse_keyword(Keyword::ADD) { - parser.expect_keyword(Keyword::STORAGE_LOCATION)?; + parser.expect_keyword_is(Keyword::STORAGE_LOCATION)?; parser.expect_token(&Token::Eq)?; - parser.expect_token(&Token::LParen)?; - let loc = parse_external_volume_storage_location(parser)?; - parser.expect_token(&Token::RParen)?; - AlterExternalVolumeOperation::AddStorageLocation(loc) + AlterExternalVolumeOperation::AddStorageLocation(parser.parse_key_value_options(true, &[])?) } else if parser.parse_keyword(Keyword::SET) { - parser.expect_keyword(Keyword::ALLOW_WRITES)?; + parser.expect_keyword_is(Keyword::ALLOW_WRITES)?; parser.expect_token(&Token::Eq)?; AlterExternalVolumeOperation::SetAllowWrites(parser.parse_boolean_string()?) } else if parser.parse_keyword(Keyword::REMOVE) { - parser.expect_keyword(Keyword::STORAGE_LOCATION)?; + parser.expect_keyword_is(Keyword::STORAGE_LOCATION)?; let loc_name = parser.parse_literal_string()?; AlterExternalVolumeOperation::RemoveStorageLocation(loc_name) } else { @@ -2199,13 +2091,6 @@ fn parse_alter_external_volume(parser: &mut Parser) -> Result` -fn parse_drop_external_volume(parser: &mut Parser) -> Result { - let if_exists = parser.parse_keywords(&[Keyword::IF, Keyword::EXISTS]); - let name = parser.parse_object_name(false)?; - Ok(Statement::DropExternalVolume { name, if_exists }) -} - /// Parse `DESC[RIBE] EXTERNAL VOLUME ` fn parse_describe_external_volume(parser: &mut Parser) -> Result { let name = parser.parse_object_name(false)?; diff --git a/src/keywords.rs b/src/keywords.rs index d3163c6b2..d3904a4cb 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -570,7 +570,6 @@ define_keywords!( KEYS, KEY_BLOCK_SIZE, KILL, - KMS_KEY_ID, LAG, LAMBDA, LANGUAGE, @@ -1006,13 +1005,9 @@ define_keywords!( STDOUT, STEP, STORAGE, - STORAGE_AWS_EXTERNAL_ID, - STORAGE_AWS_ROLE_ARN, - STORAGE_BASE_URL, STORAGE_INTEGRATION, STORAGE_LOCATION, STORAGE_LOCATIONS, - STORAGE_PROVIDER, STORAGE_SERIALIZATION_POLICY, STORED, STRAIGHT_JOIN, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index ac1b5e376..a4c23b2a0 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7581,6 +7581,8 @@ impl<'a> Parser<'a> { ObjectType::User } else if self.parse_keyword(Keyword::STREAM) { ObjectType::Stream + } else if self.parse_keywords(&[Keyword::EXTERNAL, Keyword::VOLUME]) { + ObjectType::ExternalVolume } else if self.parse_keyword(Keyword::FUNCTION) { return self.parse_drop_function().map(Into::into); } else if self.parse_keyword(Keyword::POLICY) { @@ -7608,7 +7610,7 @@ impl<'a> Parser<'a> { }; } else { return self.expected_ref( - "COLLATION, CONNECTOR, DATABASE, EXTENSION, FUNCTION, INDEX, OPERATOR, POLICY, PROCEDURE, ROLE, SCHEMA, SECRET, SEQUENCE, STAGE, TABLE, TRIGGER, TYPE, VIEW, MATERIALIZED VIEW or USER after DROP", + "COLLATION, CONNECTOR, DATABASE, EXTENSION, EXTERNAL VOLUME, FUNCTION, INDEX, OPERATOR, POLICY, PROCEDURE, ROLE, SCHEMA, SECRET, SEQUENCE, STAGE, TABLE, TRIGGER, TYPE, VIEW, MATERIALIZED VIEW or USER after DROP", self.peek_token_ref(), ); }; diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index c3d05275a..7c0471a32 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -4915,13 +4915,24 @@ fn test_select_dollar_column_from_stage() { snowflake().verified_stmt("SELECT $1, $2 FROM @mystage1(file_format => 'myformat')"); } +/// Return the string value of a single-valued option by name, if present. +fn ext_vol_option<'a>(options: &'a [KeyValueOption], name: &str) -> Option<&'a str> { + options + .iter() + .find(|o| o.option_name == name) + .and_then(|o| match &o.option_value { + KeyValueOptionKind::Single(v) => match &v.value { + Value::SingleQuotedString(s) => Some(s.as_str()), + _ => None, + }, + _ => None, + }) +} + #[test] fn test_create_external_volume_basic() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/path/'))", - ); + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/path/'))"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { or_replace, @@ -4935,11 +4946,13 @@ fn test_create_external_volume_basic() { assert!(!if_not_exists); assert_eq!("my_vol", name.to_string()); assert_eq!(1, storage_locations.len()); - assert_eq!("loc1", storage_locations[0].name); - assert_eq!("S3", storage_locations[0].storage_provider); - assert_eq!("s3://bucket/path/", storage_locations[0].storage_base_url); - assert!(storage_locations[0].storage_aws_role_arn.is_none()); - assert!(storage_locations[0].encryption.is_none()); + let loc = &storage_locations[0].options; + assert_eq!(Some("loc1"), ext_vol_option(loc, "NAME")); + assert_eq!(Some("S3"), ext_vol_option(loc, "STORAGE_PROVIDER")); + assert_eq!( + Some("s3://bucket/path/"), + ext_vol_option(loc, "STORAGE_BASE_URL") + ); assert!(allow_writes.is_none()); assert!(comment.is_none()); } @@ -4949,11 +4962,8 @@ fn test_create_external_volume_basic() { #[test] fn test_create_external_volume_or_replace() { - let sql = concat!( - "CREATE OR REPLACE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/'))", - ); + let sql = "CREATE OR REPLACE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/'))"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { or_replace, .. } => { assert!(or_replace); @@ -4964,11 +4974,8 @@ fn test_create_external_volume_or_replace() { #[test] fn test_create_external_volume_if_not_exists() { - let sql = concat!( - "CREATE EXTERNAL VOLUME IF NOT EXISTS my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/'))", - ); + let sql = "CREATE EXTERNAL VOLUME IF NOT EXISTS my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/'))"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { if_not_exists, .. } => { assert!(if_not_exists); @@ -4979,23 +4986,26 @@ fn test_create_external_volume_if_not_exists() { #[test] fn test_create_external_volume_multi_location() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket1/'), ", - "(NAME = 'loc2' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket2/' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/myrole'))", - ); + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket1/'), \ + (NAME='loc2' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket2/' \ + STORAGE_AWS_ROLE_ARN='arn:aws:iam::role/myrole'))"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { storage_locations, .. } => { assert_eq!(2, storage_locations.len()); - assert_eq!("loc1", storage_locations[0].name); - assert_eq!("loc2", storage_locations[1].name); assert_eq!( - Some("arn:aws:iam::role/myrole".to_string()), - storage_locations[1].storage_aws_role_arn + Some("loc1"), + ext_vol_option(&storage_locations[0].options, "NAME") + ); + assert_eq!( + Some("loc2"), + ext_vol_option(&storage_locations[1].options, "NAME") + ); + assert_eq!( + Some("arn:aws:iam::role/myrole"), + ext_vol_option(&storage_locations[1].options, "STORAGE_AWS_ROLE_ARN") ); } _ => unreachable!(), @@ -5004,39 +5014,38 @@ fn test_create_external_volume_multi_location() { #[test] fn test_create_external_volume_with_encryption_sse_s3() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", - "ENCRYPTION = (TYPE = 'AWS_SSE_S3')))", + snowflake().verified_stmt( + "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/' \ + ENCRYPTION=(TYPE='AWS_SSE_S3')))", ); - match snowflake().verified_stmt(sql) { - Statement::CreateExternalVolume { - storage_locations, .. - } => { - let enc = storage_locations[0].encryption.as_ref().unwrap(); - assert_eq!("AWS_SSE_S3", enc.kind); - assert!(enc.kms_key_id.is_none()); - } - _ => unreachable!(), - } } #[test] fn test_create_external_volume_with_encryption_kms() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", - "ENCRYPTION = (TYPE = 'AWS_SSE_KMS' KMS_KEY_ID = 'my-key-id')))", - ); + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/' \ + ENCRYPTION=(TYPE='AWS_SSE_KMS' KMS_KEY_ID='my-key-id')))"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { storage_locations, .. } => { - let enc = storage_locations[0].encryption.as_ref().unwrap(); - assert_eq!("AWS_SSE_KMS", enc.kind); - assert_eq!(Some("my-key-id".to_string()), enc.kms_key_id); + // ENCRYPTION is parsed as a nested key-value option list. + let enc = storage_locations[0] + .options + .iter() + .find(|o| o.option_name == "ENCRYPTION") + .expect("ENCRYPTION option present"); + match &enc.option_value { + KeyValueOptionKind::KeyValueOptions(inner) => { + assert_eq!(Some("AWS_SSE_KMS"), ext_vol_option(&inner.options, "TYPE")); + assert_eq!( + Some("my-key-id"), + ext_vol_option(&inner.options, "KMS_KEY_ID") + ); + } + _ => unreachable!("ENCRYPTION should be a nested option list"), + } } _ => unreachable!(), } @@ -5044,35 +5053,21 @@ fn test_create_external_volume_with_encryption_kms() { #[test] fn test_create_external_volume_with_encryption_none() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", - "ENCRYPTION = (TYPE = 'NONE')))", + snowflake().verified_stmt( + "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/' \ + ENCRYPTION=(TYPE='NONE')))", ); - match snowflake().verified_stmt(sql) { - Statement::CreateExternalVolume { - storage_locations, .. - } => { - let enc = storage_locations[0].encryption.as_ref().unwrap(); - assert_eq!("NONE", enc.kind); - } - _ => unreachable!(), - } } #[test] fn test_create_external_volume_full() { - let sql = concat!( - "CREATE OR REPLACE EXTERNAL VOLUME IF NOT EXISTS my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r' ", - "STORAGE_AWS_EXTERNAL_ID = 'ext-id' ", - "ENCRYPTION = (TYPE = 'AWS_SSE_KMS' KMS_KEY_ID = 'key'))) ", - "ALLOW_WRITES = TRUE ", - "COMMENT = 'my comment'", - ); + let sql = "CREATE OR REPLACE EXTERNAL VOLUME IF NOT EXISTS my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/' \ + STORAGE_AWS_ROLE_ARN='arn:aws:iam::role/r' \ + STORAGE_AWS_EXTERNAL_ID='ext-id' \ + ENCRYPTION=(TYPE='AWS_SSE_KMS' KMS_KEY_ID='key'))) \ + ALLOW_WRITES = TRUE COMMENT = 'my comment'"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { or_replace, @@ -5086,8 +5081,8 @@ fn test_create_external_volume_full() { assert!(if_not_exists); assert_eq!(1, storage_locations.len()); assert_eq!( - Some("ext-id".to_string()), - storage_locations[0].storage_aws_external_id + Some("ext-id"), + ext_vol_option(&storage_locations[0].options, "STORAGE_AWS_EXTERNAL_ID") ); assert_eq!(Some(true), allow_writes); assert_eq!(Some("my comment".to_string()), comment); @@ -5098,12 +5093,9 @@ fn test_create_external_volume_full() { #[test] fn test_create_external_volume_allow_writes_false() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/')) ", - "ALLOW_WRITES = FALSE", - ); + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/')) \ + ALLOW_WRITES = FALSE"; match snowflake().verified_stmt(sql) { Statement::CreateExternalVolume { allow_writes, .. } => { assert_eq!(Some(false), allow_writes); @@ -5113,11 +5105,95 @@ fn test_create_external_volume_allow_writes_false() { } #[test] -fn test_alter_external_volume_add_storage_location() { - let sql = concat!( - "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ", - "(NAME = 'loc2' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket2/')", +fn test_create_external_volume_comma_separated_fields() { + // Options within a location may be comma-separated; the comma delimiter + // is preserved on round-trip. + snowflake().verified_stmt( + "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1', STORAGE_PROVIDER='S3', STORAGE_BASE_URL='s3://bucket/', \ + STORAGE_AWS_ROLE_ARN='arn:aws:iam::role/r'))", + ); +} + +#[test] +fn test_create_external_volume_flexible_field_ordering() { + // Field order within a location is preserved as written (not normalized). + snowflake().verified_stmt( + "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' \ + STORAGE_AWS_ROLE_ARN='arn:aws:iam::role/r' STORAGE_BASE_URL='s3://bucket/'))", + ); +} + +#[test] +fn test_create_external_volume_spaces_around_equals_normalized() { + // Snowflake accepts spaces around `=`; they are removed on round-trip, + // matching the rest of the dialect's key-value option rendering. + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/'))"; + let canonical = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/'))"; + snowflake().one_statement_parses_to(sql, canonical); +} + +#[test] +fn test_create_external_volume_minimal_location_accepted() { + // Parsing is syntax-only: a location with no STORAGE_BASE_URL is accepted + // (semantic validation is left to the consumer). + snowflake().verified_stmt( + "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3'))", + ); +} + +#[test] +fn test_create_external_volume_escaped_single_quotes() { + // Single quotes inside string values round-trip through escaping. + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='lo''c1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/')) \ + COMMENT = 'it''s mine'"; + match snowflake().verified_stmt(sql) { + Statement::CreateExternalVolume { + storage_locations, + comment, + .. + } => { + assert_eq!( + Some("lo'c1"), + ext_vol_option(&storage_locations[0].options, "NAME") + ); + assert_eq!(Some("it's mine".to_string()), comment); + } + _ => unreachable!(), + } +} + +#[test] +fn test_create_external_volume_empty_storage_locations() { + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = ()"; + snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject empty STORAGE_LOCATIONS"); +} + +#[test] +fn test_create_external_volume_allow_writes_non_boolean() { + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/')) \ + ALLOW_WRITES = 1"; + let err = snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject non-boolean ALLOW_WRITES"); + assert!( + err.to_string().contains("TRUE or FALSE"), + "unexpected error: {err}" ); +} + +#[test] +fn test_alter_external_volume_add_storage_location() { + let sql = "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = \ + (NAME='loc2' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket2/')"; match snowflake().verified_stmt(sql) { Statement::AlterExternalVolume { name, @@ -5128,8 +5204,8 @@ fn test_alter_external_volume_add_storage_location() { assert!(!if_exists); match operation { AlterExternalVolumeOperation::AddStorageLocation(loc) => { - assert_eq!("loc2", loc.name); - assert_eq!("S3", loc.storage_provider); + assert_eq!(Some("loc2"), ext_vol_option(&loc.options, "NAME")); + assert_eq!(Some("S3"), ext_vol_option(&loc.options, "STORAGE_PROVIDER")); } _ => unreachable!(), } @@ -5138,10 +5214,22 @@ fn test_alter_external_volume_add_storage_location() { } } +#[test] +fn test_alter_external_volume_add_storage_location_full() { + // The ADD path reuses the same option parser, so exercise the optional + // fields (external id + encryption) through it too. + snowflake().verified_stmt( + "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = \ + (NAME='loc2' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket2/' \ + STORAGE_AWS_ROLE_ARN='arn:aws:iam::role/r' \ + STORAGE_AWS_EXTERNAL_ID='ext-id' \ + ENCRYPTION=(TYPE='AWS_SSE_KMS' KMS_KEY_ID='key'))", + ); +} + #[test] fn test_alter_external_volume_set_allow_writes() { - let sql = "ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = TRUE"; - match snowflake().verified_stmt(sql) { + match snowflake().verified_stmt("ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = TRUE") { Statement::AlterExternalVolume { operation, .. } => { assert_eq!( AlterExternalVolumeOperation::SetAllowWrites(true), @@ -5151,8 +5239,7 @@ fn test_alter_external_volume_set_allow_writes() { _ => unreachable!(), } - let sql = "ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = FALSE"; - match snowflake().verified_stmt(sql) { + match snowflake().verified_stmt("ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = FALSE") { Statement::AlterExternalVolume { operation, .. } => { assert_eq!( AlterExternalVolumeOperation::SetAllowWrites(false), @@ -5165,8 +5252,9 @@ fn test_alter_external_volume_set_allow_writes() { #[test] fn test_alter_external_volume_if_exists() { - let sql = "ALTER EXTERNAL VOLUME IF EXISTS my_vol SET ALLOW_WRITES = TRUE"; - match snowflake().verified_stmt(sql) { + match snowflake() + .verified_stmt("ALTER EXTERNAL VOLUME IF EXISTS my_vol SET ALLOW_WRITES = TRUE") + { Statement::AlterExternalVolume { if_exists, .. } => { assert!(if_exists); } @@ -5176,8 +5264,7 @@ fn test_alter_external_volume_if_exists() { #[test] fn test_alter_external_volume_remove_storage_location() { - let sql = "ALTER EXTERNAL VOLUME my_vol REMOVE STORAGE_LOCATION 'loc1'"; - match snowflake().verified_stmt(sql) { + match snowflake().verified_stmt("ALTER EXTERNAL VOLUME my_vol REMOVE STORAGE_LOCATION 'loc1'") { Statement::AlterExternalVolume { operation, .. } => { assert_eq!( AlterExternalVolumeOperation::RemoveStorageLocation("loc1".to_string()), @@ -5188,13 +5275,40 @@ fn test_alter_external_volume_remove_storage_location() { } } +#[test] +fn test_alter_external_volume_allow_writes_non_boolean() { + let err = snowflake() + .parse_sql_statements("ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = 1") + .expect_err("parser must reject non-boolean ALLOW_WRITES"); + assert!( + err.to_string().contains("TRUE or FALSE"), + "unexpected error: {err}" + ); +} + +#[test] +fn test_alter_external_volume_missing_operation() { + let err = snowflake() + .parse_sql_statements("ALTER EXTERNAL VOLUME my_vol") + .expect_err("parser must reject ALTER EXTERNAL VOLUME without an operation"); + assert!( + err.to_string().contains("ADD, SET, or REMOVE"), + "unexpected error: {err}" + ); +} + #[test] fn test_drop_external_volume() { - let sql = "DROP EXTERNAL VOLUME my_vol"; - match snowflake().verified_stmt(sql) { - Statement::DropExternalVolume { name, if_exists } => { - assert_eq!("my_vol", name.to_string()); + match snowflake().verified_stmt("DROP EXTERNAL VOLUME my_vol") { + Statement::Drop { + object_type, + if_exists, + names, + .. + } => { + assert_eq!(ObjectType::ExternalVolume, object_type); assert!(!if_exists); + assert_eq!("my_vol", names[0].to_string()); } _ => unreachable!(), } @@ -5202,9 +5316,13 @@ fn test_drop_external_volume() { #[test] fn test_drop_external_volume_if_exists() { - let sql = "DROP EXTERNAL VOLUME IF EXISTS my_vol"; - match snowflake().verified_stmt(sql) { - Statement::DropExternalVolume { if_exists, .. } => { + match snowflake().verified_stmt("DROP EXTERNAL VOLUME IF EXISTS my_vol") { + Statement::Drop { + object_type, + if_exists, + .. + } => { + assert_eq!(ObjectType::ExternalVolume, object_type); assert!(if_exists); } _ => unreachable!(), @@ -5213,8 +5331,7 @@ fn test_drop_external_volume_if_exists() { #[test] fn test_describe_external_volume() { - let sql = "DESCRIBE EXTERNAL VOLUME my_vol"; - match snowflake().verified_stmt(sql) { + match snowflake().verified_stmt("DESCRIBE EXTERNAL VOLUME my_vol") { Statement::DescribeExternalVolume { name } => { assert_eq!("my_vol", name.to_string()); } @@ -5222,39 +5339,10 @@ fn test_describe_external_volume() { } } -#[test] -fn test_show_external_volumes() { - let sql = "SHOW EXTERNAL VOLUMES"; - match snowflake().verified_stmt(sql) { - Statement::ShowExternalVolumes { filter } => { - assert!(filter.is_none()); - } - _ => unreachable!(), - } -} - -#[test] -fn test_show_external_volumes_like() { - let sql = "SHOW EXTERNAL VOLUMES LIKE 'my_%'"; - match snowflake().verified_stmt(sql) { - Statement::ShowExternalVolumes { filter } => { - assert!(filter.is_some()); - match filter.unwrap() { - ShowStatementFilter::Like(pattern) => { - assert_eq!("my_%", pattern); - } - _ => unreachable!(), - } - } - _ => unreachable!(), - } -} - #[test] fn test_desc_external_volume() { - let sql = "DESC EXTERNAL VOLUME my_vol"; let canonical = "DESCRIBE EXTERNAL VOLUME my_vol"; - match snowflake().one_statement_parses_to(sql, canonical) { + match snowflake().one_statement_parses_to("DESC EXTERNAL VOLUME my_vol", canonical) { Statement::DescribeExternalVolume { name } => { assert_eq!("my_vol", name.to_string()); } @@ -5263,246 +5351,24 @@ fn test_desc_external_volume() { } #[test] -fn test_create_external_volume_comma_separated_fields() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1', STORAGE_PROVIDER = 'S3', STORAGE_BASE_URL = 's3://bucket/', ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", - ); - let canonical = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", - ); - match snowflake().one_statement_parses_to(sql, canonical) { - Statement::CreateExternalVolume { - storage_locations, .. - } => { - assert_eq!(1, storage_locations.len()); - assert_eq!("loc1", storage_locations[0].name); - assert_eq!("S3", storage_locations[0].storage_provider); - assert_eq!("s3://bucket/", storage_locations[0].storage_base_url); - assert_eq!( - Some("arn:aws:iam::role/r".to_string()), - storage_locations[0].storage_aws_role_arn - ); - } - _ => unreachable!(), - } -} - -#[test] -fn test_create_external_volume_flexible_field_ordering() { - // Real Snowflake accepts STORAGE_BASE_URL anywhere after STORAGE_PROVIDER. - // Here STORAGE_AWS_ROLE_ARN appears before STORAGE_BASE_URL. - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r' ", - "STORAGE_BASE_URL = 's3://bucket/'))", - ); - let canonical = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", - ); - match snowflake().one_statement_parses_to(sql, canonical) { - Statement::CreateExternalVolume { - storage_locations, .. - } => { - assert_eq!(1, storage_locations.len()); - assert_eq!("loc1", storage_locations[0].name); - assert_eq!("S3", storage_locations[0].storage_provider); - assert_eq!("s3://bucket/", storage_locations[0].storage_base_url); - assert_eq!( - Some("arn:aws:iam::role/r".to_string()), - storage_locations[0].storage_aws_role_arn - ); - } - _ => unreachable!(), - } -} - -#[test] -fn test_create_external_volume_missing_storage_base_url() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r'))", - ); - let err = snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject STORAGE_LOCATION without STORAGE_BASE_URL"); - assert!( - err.to_string().contains("STORAGE_BASE_URL"), - "unexpected error: {err}" - ); -} - -#[test] -fn test_create_external_volume_duplicate_storage_base_url() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' ", - "STORAGE_BASE_URL = 's3://bucket/' ", - "STORAGE_BASE_URL = 's3://other/'))", - ); - let err = snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject duplicate STORAGE_BASE_URL"); - assert!( - err.to_string().contains("duplicate STORAGE_BASE_URL"), - "unexpected error: {err}" - ); -} - -#[test] -fn test_create_external_volume_duplicate_optional_fields() { - // Each optional field is rejected when repeated within a STORAGE_LOCATION. - let cases = [ - ( - "STORAGE_AWS_ROLE_ARN = 'a' STORAGE_AWS_ROLE_ARN = 'b'", - "duplicate STORAGE_AWS_ROLE_ARN", - ), - ( - "STORAGE_AWS_EXTERNAL_ID = 'a' STORAGE_AWS_EXTERNAL_ID = 'b'", - "duplicate STORAGE_AWS_EXTERNAL_ID", - ), - ( - "ENCRYPTION = (TYPE = 'NONE') ENCRYPTION = (TYPE = 'NONE')", - "duplicate ENCRYPTION", - ), - ]; - for (fields, expected) in cases { - let sql = format!( - "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = (\ - (NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/' {fields}))" - ); - let err = snowflake() - .parse_sql_statements(&sql) - .expect_err("parser must reject duplicate field"); - assert!( - err.to_string().contains(expected), - "expected `{expected}`, got: {err}" - ); - } -} - -#[test] -fn test_create_external_volume_empty_storage_locations() { - let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = ()"; - snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject empty STORAGE_LOCATIONS"); -} - -#[test] -fn test_create_external_volume_allow_writes_non_boolean() { - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'loc1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/')) ", - "ALLOW_WRITES = 1", - ); - let err = snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject non-boolean ALLOW_WRITES"); - assert!( - err.to_string().contains("TRUE or FALSE"), - "unexpected error: {err}" - ); -} - -#[test] -fn test_create_external_volume_escaped_single_quotes() { - // Single quotes inside string values round-trip through escaping. - let sql = concat!( - "CREATE EXTERNAL VOLUME my_vol ", - "STORAGE_LOCATIONS = (", - "(NAME = 'lo''c1' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket/')) ", - "COMMENT = 'it''s mine'", - ); - match snowflake().verified_stmt(sql) { - Statement::CreateExternalVolume { - storage_locations, - comment, - .. - } => { - assert_eq!("lo'c1", storage_locations[0].name); - assert_eq!(Some("it's mine".to_string()), comment); +fn test_show_external_volumes() { + match snowflake().verified_stmt("SHOW EXTERNAL VOLUMES") { + Statement::ShowExternalVolumes { filter } => { + assert!(filter.is_none()); } _ => unreachable!(), } } #[test] -fn test_alter_external_volume_add_storage_location_full() { - // The ADD path reuses the storage-location parser, so exercise the - // optional fields (external id + encryption) through it too. - let sql = concat!( - "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ", - "(NAME = 'loc2' STORAGE_PROVIDER = 'S3' STORAGE_BASE_URL = 's3://bucket2/' ", - "STORAGE_AWS_ROLE_ARN = 'arn:aws:iam::role/r' ", - "STORAGE_AWS_EXTERNAL_ID = 'ext-id' ", - "ENCRYPTION = (TYPE = 'AWS_SSE_KMS' KMS_KEY_ID = 'key'))", - ); - match snowflake().verified_stmt(sql) { - Statement::AlterExternalVolume { operation, .. } => match operation { - AlterExternalVolumeOperation::AddStorageLocation(loc) => { - assert_eq!("loc2", loc.name); - assert_eq!(Some("ext-id".to_string()), loc.storage_aws_external_id); - let enc = loc.encryption.as_ref().unwrap(); - assert_eq!("AWS_SSE_KMS", enc.kind); - assert_eq!(Some("key".to_string()), enc.kms_key_id); +fn test_show_external_volumes_like() { + match snowflake().verified_stmt("SHOW EXTERNAL VOLUMES LIKE 'my_%'") { + Statement::ShowExternalVolumes { filter } => match filter { + Some(ShowStatementFilter::Like(pattern)) => { + assert_eq!("my_%", pattern); } _ => unreachable!(), }, _ => unreachable!(), } } - -#[test] -fn test_alter_external_volume_add_missing_storage_base_url() { - // The ADD path enforces the same required-field rule as CREATE. - let sql = concat!( - "ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ", - "(NAME = 'loc2' STORAGE_PROVIDER = 'S3')", - ); - let err = snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject ADD STORAGE_LOCATION without STORAGE_BASE_URL"); - assert!( - err.to_string().contains("STORAGE_BASE_URL"), - "unexpected error: {err}" - ); -} - -#[test] -fn test_alter_external_volume_allow_writes_non_boolean() { - let sql = "ALTER EXTERNAL VOLUME my_vol SET ALLOW_WRITES = 1"; - let err = snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject non-boolean ALLOW_WRITES"); - assert!( - err.to_string().contains("TRUE or FALSE"), - "unexpected error: {err}" - ); -} - -#[test] -fn test_alter_external_volume_missing_operation() { - let sql = "ALTER EXTERNAL VOLUME my_vol"; - let err = snowflake() - .parse_sql_statements(sql) - .expect_err("parser must reject ALTER EXTERNAL VOLUME without an operation"); - assert!( - err.to_string().contains("ADD, SET, or REMOVE"), - "unexpected error: {err}" - ); -} From c2d77718fe842c2fb95e298d3c70798f2f8cce6d Mon Sep 17 00:00:00 2001 From: sabir Date: Fri, 3 Jul 2026 14:27:33 +0200 Subject: [PATCH 3/4] Fix clippy collapsible_match in EXTERNAL VOLUME test Co-Authored-By: Claude Opus 4.8 --- tests/sqlparser_snowflake.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index 7c0471a32..af6c587b0 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -5363,12 +5363,11 @@ fn test_show_external_volumes() { #[test] fn test_show_external_volumes_like() { match snowflake().verified_stmt("SHOW EXTERNAL VOLUMES LIKE 'my_%'") { - Statement::ShowExternalVolumes { filter } => match filter { - Some(ShowStatementFilter::Like(pattern)) => { - assert_eq!("my_%", pattern); - } - _ => unreachable!(), - }, + Statement::ShowExternalVolumes { + filter: Some(ShowStatementFilter::Like(pattern)), + } => { + assert_eq!("my_%", pattern); + } _ => unreachable!(), } } From c76689d8ff08b8f0fe73ed8882b555e908a3524e Mon Sep 17 00:00:00 2001 From: sabir Date: Fri, 3 Jul 2026 22:09:18 +0200 Subject: [PATCH 4/4] EXTERNAL VOLUME: preserve DESC alias, flexible option order, reject empty locations Address review findings: preserve the DESC/DESCRIBE spelling via a DescribeAlias field so DESC EXTERNAL VOLUME round-trips verbatim like every other DESC form; accept ALLOW_WRITES and COMMENT in either order, matching parse_create_database; reject an empty storage location (STORAGE_LOCATIONS = (()) previously parsed while () was rejected) in both CREATE and ALTER ADD via a shared helper; and use parse_comma_separated instead of a hand-rolled comma loop. Co-Authored-By: Claude Fable 5 --- src/ast/mod.rs | 9 ++++- src/dialect/snowflake.rs | 78 ++++++++++++++++++++++-------------- tests/sqlparser_snowflake.rs | 57 ++++++++++++++++++++++++-- 3 files changed, 109 insertions(+), 35 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index ea256d6f1..2b7a4319c 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -4569,6 +4569,8 @@ pub enum Statement { /// DESC[RIBE] EXTERNAL VOLUME /// ``` DescribeExternalVolume { + /// The keyword used, `DESC` or `DESCRIBE`. + describe_alias: DescribeAlias, /// External volume name. name: ObjectName, }, @@ -6345,8 +6347,11 @@ impl fmt::Display for Statement { if_exists = if *if_exists { "IF EXISTS " } else { "" }, ) } - Statement::DescribeExternalVolume { name } => { - write!(f, "DESCRIBE EXTERNAL VOLUME {name}") + Statement::DescribeExternalVolume { + describe_alias, + name, + } => { + write!(f, "{describe_alias} EXTERNAL VOLUME {name}") } Statement::ShowExternalVolumes { filter } => { write!(f, "SHOW EXTERNAL VOLUMES")?; diff --git a/src/dialect/snowflake.rs b/src/dialect/snowflake.rs index 157541119..9e63ecbc0 100644 --- a/src/dialect/snowflake.rs +++ b/src/dialect/snowflake.rs @@ -29,8 +29,8 @@ use crate::ast::helpers::stmt_data_loading::{ use crate::ast::{ AlterExternalVolumeOperation, AlterTable, AlterTableOperation, AlterTableType, CatalogSyncNamespaceMode, ColumnOption, ColumnPolicy, ColumnPolicyProperty, ContactEntry, - CopyIntoSnowflakeKind, CreateTable, CreateTableLikeKind, DollarQuotedString, Ident, - IdentityParameters, IdentityProperty, IdentityPropertyFormatKind, IdentityPropertyKind, + CopyIntoSnowflakeKind, CreateTable, CreateTableLikeKind, DescribeAlias, DollarQuotedString, + Ident, IdentityParameters, IdentityProperty, IdentityPropertyFormatKind, IdentityPropertyKind, IdentityPropertyOrder, InitializeKind, Insert, MultiTableInsertIntoClause, MultiTableInsertType, MultiTableInsertValue, MultiTableInsertValues, MultiTableInsertWhenClause, ObjectName, ObjectNamePart, RefreshModeKind, RowAccessPolicy, @@ -287,13 +287,15 @@ impl Dialect for SnowflakeDialect { return Some(parse_alter_session(parser, set)); } - if parser - .parse_one_of_keywords(&[Keyword::DESC, Keyword::DESCRIBE]) - .is_some() - { + if let Some(kw) = parser.parse_one_of_keywords(&[Keyword::DESC, Keyword::DESCRIBE]) { if parser.parse_keywords(&[Keyword::EXTERNAL, Keyword::VOLUME]) { // DESC[RIBE] EXTERNAL VOLUME - return Some(parse_describe_external_volume(parser)); + let describe_alias = if kw == Keyword::DESC { + DescribeAlias::Desc + } else { + DescribeAlias::Describe + }; + return Some(parse_describe_external_volume(describe_alias, parser)); } // not EXTERNAL VOLUME — put back DESC/DESCRIBE parser.prev_token(); @@ -2013,10 +2015,9 @@ fn parse_multi_table_insert_when_clauses( /// Parse `CREATE [OR REPLACE] EXTERNAL VOLUME [IF NOT EXISTS] ...` /// -/// Each storage location is a parenthesized `KEY = value` option list; the -/// options (and the `ENCRYPTION = (...)` sub-list) are parsed generically via -/// [`Parser::parse_key_value_options`], so field order and the exact option -/// set are left to the caller rather than validated here. +/// Each storage location is parsed by [`parse_external_volume_storage_location`]; +/// the trailing `ALLOW_WRITES` and `COMMENT` properties are accepted in any +/// order. fn parse_create_external_volume( or_replace: bool, parser: &mut Parser, @@ -2028,26 +2029,22 @@ fn parse_create_external_volume( parser.expect_token(&Token::Eq)?; parser.expect_token(&Token::LParen)?; - let mut storage_locations = vec![]; - loop { - storage_locations.push(parser.parse_key_value_options(true, &[])?); - if !parser.consume_token(&Token::Comma) { - break; - } - } + let storage_locations = parser.parse_comma_separated(parse_external_volume_storage_location)?; parser.expect_token(&Token::RParen)?; let mut allow_writes = None; let mut comment = None; - if parser.parse_keyword(Keyword::ALLOW_WRITES) { - parser.expect_token(&Token::Eq)?; - allow_writes = Some(parser.parse_boolean_string()?); - } - - if parser.parse_keyword(Keyword::COMMENT) { - parser.expect_token(&Token::Eq)?; - comment = Some(parser.parse_comment_value()?); + loop { + if parser.parse_keyword(Keyword::ALLOW_WRITES) { + parser.expect_token(&Token::Eq)?; + allow_writes = Some(parser.parse_boolean_string()?); + } else if parser.parse_keyword(Keyword::COMMENT) { + parser.expect_token(&Token::Eq)?; + comment = Some(parser.parse_comment_value()?); + } else { + break; + } } Ok(Statement::CreateExternalVolume { @@ -2068,7 +2065,9 @@ fn parse_alter_external_volume(parser: &mut Parser) -> Result Result Result { + let location = parser.parse_key_value_options(true, &[])?; + if location.options.is_empty() { + return parser.expected("storage location options", parser.peek_token()); + } + Ok(location) +} + /// Parse `DESC[RIBE] EXTERNAL VOLUME ` -fn parse_describe_external_volume(parser: &mut Parser) -> Result { +fn parse_describe_external_volume( + describe_alias: DescribeAlias, + parser: &mut Parser, +) -> Result { let name = parser.parse_object_name(false)?; - Ok(Statement::DescribeExternalVolume { name }) + Ok(Statement::DescribeExternalVolume { + describe_alias, + name, + }) } /// Parse `SHOW EXTERNAL VOLUMES [LIKE '']` diff --git a/tests/sqlparser_snowflake.rs b/tests/sqlparser_snowflake.rs index af6c587b0..ee1ca862c 100644 --- a/tests/sqlparser_snowflake.rs +++ b/tests/sqlparser_snowflake.rs @@ -5176,6 +5176,36 @@ fn test_create_external_volume_empty_storage_locations() { .expect_err("parser must reject empty STORAGE_LOCATIONS"); } +#[test] +fn test_create_external_volume_empty_storage_location() { + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = (())"; + snowflake() + .parse_sql_statements(sql) + .expect_err("parser must reject an empty storage location"); +} + +#[test] +fn test_create_external_volume_comment_before_allow_writes() { + // ALLOW_WRITES and COMMENT parse in either order; display order is canonical. + let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/')) \ + COMMENT = 'my comment' ALLOW_WRITES = TRUE"; + let canonical = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ + ((NAME='loc1' STORAGE_PROVIDER='S3' STORAGE_BASE_URL='s3://bucket/')) \ + ALLOW_WRITES = TRUE COMMENT = 'my comment'"; + match snowflake().one_statement_parses_to(sql, canonical) { + Statement::CreateExternalVolume { + allow_writes, + comment, + .. + } => { + assert_eq!(Some(true), allow_writes); + assert_eq!(Some("my comment".to_string()), comment); + } + _ => unreachable!(), + } +} + #[test] fn test_create_external_volume_allow_writes_non_boolean() { let sql = "CREATE EXTERNAL VOLUME my_vol STORAGE_LOCATIONS = \ @@ -5275,6 +5305,17 @@ fn test_alter_external_volume_remove_storage_location() { } } +#[test] +fn test_alter_external_volume_add_empty_storage_location() { + let err = snowflake() + .parse_sql_statements("ALTER EXTERNAL VOLUME my_vol ADD STORAGE_LOCATION = ()") + .expect_err("parser must reject an empty storage location"); + assert!( + err.to_string().contains("storage location options"), + "unexpected error: {err}" + ); +} + #[test] fn test_alter_external_volume_allow_writes_non_boolean() { let err = snowflake() @@ -5332,7 +5373,11 @@ fn test_drop_external_volume_if_exists() { #[test] fn test_describe_external_volume() { match snowflake().verified_stmt("DESCRIBE EXTERNAL VOLUME my_vol") { - Statement::DescribeExternalVolume { name } => { + Statement::DescribeExternalVolume { + describe_alias, + name, + } => { + assert_eq!(DescribeAlias::Describe, describe_alias); assert_eq!("my_vol", name.to_string()); } _ => unreachable!(), @@ -5341,9 +5386,13 @@ fn test_describe_external_volume() { #[test] fn test_desc_external_volume() { - let canonical = "DESCRIBE EXTERNAL VOLUME my_vol"; - match snowflake().one_statement_parses_to("DESC EXTERNAL VOLUME my_vol", canonical) { - Statement::DescribeExternalVolume { name } => { + // The DESC spelling is preserved on round-trip. + match snowflake().verified_stmt("DESC EXTERNAL VOLUME my_vol") { + Statement::DescribeExternalVolume { + describe_alias, + name, + } => { + assert_eq!(DescribeAlias::Desc, describe_alias); assert_eq!("my_vol", name.to_string()); } _ => unreachable!(),