From 9229ae7f2446702521c46310846d3e84092fdc24 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 20:39:41 +0000 Subject: [PATCH 1/2] fix(syntax): parse Snowflake ALTER of policies and account-level objects Snowflake ALTER of these kinds used to fall back to an opaque statement, which accepted their documented negatives. They now parse into AlterObject: - Policies: the AGGREGATION, AUTHENTICATION, BACKUP, DATA MOVEMENT, FEATURE, INGRESS, JOIN, MAINTENANCE, MASKING, MULTI PARTY APPROVAL, NETWORK, PACKAGES, PASSWORD, SESSION and SNAPSHOT policies, and NETWORK RULE. ALTER SESSION POLICY no longer falls into ALTER SESSION. That covers BODY -> , AS bodies and ADD/REMOVE lists. A SET of an AS body alone is refused, as the receipts show. - FAILOVER and REPLICATION GROUP: ADD/REMOVE to the allowed lists, MOVE to a group of the same kind, PRIMARY (failover only), REFRESH, RESUME, SUSPEND [IMMEDIATE], RENAME, SET/UNSET and TAG. A SET list value runs to the next ` =`, and an integration type may be two words. - CONNECTION: PRIMARY, ENABLE/DISABLE FAILOVER, SET/UNSET COMMENT. - [ ] INTEGRATION (API, CATALOG, EXTERNAL ACCESS, NOTIFICATION, SECURITY, STORAGE): SET properties with parenthesized lists and key/value pairs, UNSET, TAG, and REFRESH of its secrets and keys. - ACCOUNT, current or named: properties, TAG, CONTACT, policies with FOR ALL ... and FORCE, RENAME TO [ SAVE_OLD_URL ], DROP OLD [ ORGANIZATION ] URL, and ADD/REMOVE ORGANIZATION USER GROUP. - ORGANIZATION: SET/UNSET properties and the event routing table, ORGANIZATION ACCOUNT, PROFILE, USER and USER GROUP. Organization-qualified account names refuse a reserved keyword as either part. Incomplete kinds, such as MASKING v1, DATA POLICY, FAILOVER v1, GROUP, PROFILE and EXTERNAL ACCESS v1, are syntax errors. --- .../src/parser/_helpers/ddl/constraints.rs | 2 +- .../src/parser/_helpers/ddl/mod.rs | 1 + .../parser/_helpers/ddl/snowflake_alter.rs | 3 + .../_helpers/ddl/snowflake_alter_account.rs | 638 ++++++++++++++++++ .../_helpers/ddl/snowflake_alter_resources.rs | 58 +- .../tests/parser/test_snowflake_alter.rs | 156 +++++ 6 files changed, 851 insertions(+), 7 deletions(-) create mode 100644 crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs index bc5a08b1a..9de0a7e70 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs @@ -1587,7 +1587,7 @@ impl Parser { TokenType::View => self.parse_alter_view_with_modifiers(None, None, None), TokenType::Index => self.parse_alter_index(), TokenType::Sequence => self.parse_alter_sequence(), - _ if self.check_identifier("SESSION") => { + _ if self.check_identifier("SESSION") && !self.check_next_identifier("POLICY") => { self.skip(); match self.parse_alter_session()? { Some(expr) => Ok(expr), diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs index bcc1c2284..1706d0e8a 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/mod.rs @@ -8,5 +8,6 @@ pub(crate) mod create_table_forms; pub(crate) mod explain; pub(crate) mod scripting; pub(crate) mod snowflake_alter; +pub(crate) mod snowflake_alter_account; pub(crate) mod snowflake_alter_resources; pub(crate) mod table_options; diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs index f92b640db..767a37551 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter.rs @@ -68,6 +68,9 @@ impl Parser { if self.match_token(TokenType::Function) || self.match_identifier("FUNCTION") { return self.parse_alter_function().map(Some); } + if let Some(alter) = self.parse_alter_snowflake_group()? { + return Ok(Some(alter)); + } if let Some(alter) = self.parse_alter_snowflake_resource()? { return Ok(Some(alter)); } diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs new file mode 100644 index 000000000..ac8f7c950 --- /dev/null +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_account.rs @@ -0,0 +1,638 @@ +//! Parser ddl: Snowflake ALTER of account-level objects: failover and replication groups, connections, integrations and organization objects. + +use crate::expressions::names::models::TableRef; +use crate::expressions::schema_objects::models::ObjectProperty; +use crate::expressions::tree::models::Expression; +use crate::expressions::types::models::Tuple; +use crate::parser::_helpers::ddl::snowflake_alter::alter_object; +use crate::parser::_helpers::ddl::snowflake_alter::property; +use crate::parser::models::Parser; +use crate::tokens::models::TokenType; +use crate::tokens::types::Result; + +/// The operands and properties of one ALTER action. +type ActionParts = (Vec, Vec); + +/// The lists a group's ADD and REMOVE change. +const ALLOWED_LISTS: &[&[&str]] = &[ + &["ALLOWED_ACCOUNTS"], + &["ALLOWED_DATABASES"], + &["ALLOWED_EXTERNAL_VOLUMES"], + &["ALLOWED_SHARES"], +]; + +impl Parser { + /// Parse a Snowflake ALTER of a group, a connection or an organization object, or None when the next words name another kind. + pub(super) fn parse_alter_snowflake_group(&mut self) -> Result> { + for kind in ["FAILOVER", "REPLICATION"] { + if self.check_identifier(kind) { + self.skip(); + self.expect_alter_words(&[&["GROUP"]])?; + return self.parse_alter_group(kind).map(Some); + } + } + if self.match_identifier("CONNECTION") { + return self.parse_alter_connection().map(Some); + } + if self.match_identifier("ACCOUNT") { + return self.parse_alter_account().map(Some); + } + if let Some(kind) = self.match_integration_kind()? { + return self.parse_alter_integration(&kind).map(Some); + } + if self.match_identifier("ORGANIZATION") { + return self.parse_alter_organization().map(Some); + } + if self.check_identifier("GROUP") || self.check_identifier("PROFILE") { + return Err(self.parse_error(format!( + "Snowflake has no ALTER {}", + self.peek_text().to_ascii_uppercase() + ))); + } + Ok(None) + } + + /// Parse ALTER FAILOVER GROUP or ALTER REPLICATION GROUP after its kind. + fn parse_alter_group(&mut self, kind: &str) -> Result { + let head = self.parse_alter_object_head()?; + let mut actions: Vec<&[&str]> = vec![ + &["REFRESH"], + &["RESUME"], + &["SUSPEND", "IMMEDIATE"], + &["SUSPEND"], + &["RENAME", "TO"], + &["SET"], + &["UNSET"], + &["ADD"], + &["REMOVE"], + &["MOVE", "DATABASES"], + &["MOVE", "SHARES"], + &["MOVE", "EXTERNAL", "VOLUMES"], + ]; + if matches!(kind, "FAILOVER") { + actions.push(&["PRIMARY"]); + } + let mut action = self.expect_alter_words(&actions)?; + let parts = match action.as_str() { + "RENAME TO" => (vec![self.parse_group_name()?], vec![]), + "SET" => (vec![], self.parse_group_set()?), + "UNSET" => (vec![], self.parse_group_unset()?), + "ADD" | "REMOVE" => { + let accounts = self.is_account_list(); + let names = match accounts { + true => self.parse_account_list()?, + false => self.parse_object_name_list()?, + }; + let preposition: &[&[&str]] = match action.as_str() { + "ADD" => &[&["TO"]], + _ => &[&["FROM"]], + }; + action.push(' '); + action.push_str(&self.expect_alter_words(preposition)?); + let list = self.expect_alter_words(ALLOWED_LISTS)?; + action.push(' '); + action.push_str(&list); + if matches!(action.as_str(), "ADD TO ALLOWED_ACCOUNTS") + && self.match_text_seq(&["IGNORE", "EDITION", "CHECK"]) + { + action.push_str(" IGNORE EDITION CHECK"); + } + (names, vec![]) + } + "PRIMARY" | "REFRESH" | "RESUME" | "SUSPEND" | "SUSPEND IMMEDIATE" => (vec![], vec![]), + _ => { + let mut operands = self.parse_object_name_list()?; + self.expect_alter_words(&[&["TO"]])?; + self.expect_alter_words(&[&[kind, "GROUP"]])?; + action.push_str(&format!(" TO {kind} GROUP")); + operands.push(self.parse_group_name()?); + (operands, vec![]) + } + }; + Ok(alter_object(&format!("{kind} GROUP"), head, action, parts)) + } + + /// Parse a group's SET: TAG pairs, or ` = [ , ... ]` properties. + fn parse_group_set(&mut self) -> Result> { + if self.check_identifier("TAG") && !self.check_next(TokenType::Eq) { + self.skip(); + return self.parse_group_tags(true); + } + let mut properties: Vec = Vec::new(); + loop { + let name = self.parse_simple_name()?; + self.expect_token(TokenType::Eq)?; + let value = self.parse_group_values()?; + properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + if self.is_at_end() || self.check(TokenType::Semicolon) { + return Ok(properties); + } + } + } + + /// Parse a group property's values: comma-separated literals or names, a name of several words such as `SECURITY INTEGRATIONS`. + fn parse_group_values(&mut self) -> Result { + let mut values: Vec = Vec::new(); + loop { + let value = self.parse_object_value()?; + let Expression::Table(mut name) = value else { + values.push(value); + if !self.match_token(TokenType::Comma) { + break; + } + continue; + }; + while !self.is_at_end() && self.check(TokenType::Var) && !self.check_next(TokenType::Eq) + { + name.name.name = format!("{} {}", name.name.name, self.peek_text()); + self.skip(); + } + values.push(Expression::Table(name)); + if !self.match_token(TokenType::Comma) { + break; + } + } + Ok(match values.len() { + 1 => values.remove(0), + _ => Expression::Tuple(Box::new(Tuple { + expressions: values, + })), + }) + } + + /// Parse TAG names, with their values when `set`, comma-separated. + fn parse_group_tags(&mut self, set: bool) -> Result> { + let mut properties: Vec = Vec::new(); + loop { + let name = self.parse_simple_qualified_name()?; + let value = if set { + self.expect_token(TokenType::Eq)?; + Some(self.parse_object_value()?) + } else { + None + }; + properties.push(property("TAG", Some(name), value)); + if !self.match_token(TokenType::Comma) { + return Ok(properties); + } + } + } + + /// Parse an UNSET: TAG names, or property names, comma-separated. + fn parse_group_unset(&mut self) -> Result> { + if self.check_identifier("TAG") && !self.is_statement_end_at(1) { + self.skip(); + return self.parse_group_tags(false); + } + let mut properties: Vec = Vec::new(); + loop { + let name = self.parse_simple_name()?; + properties.push(property(&name.name.to_ascii_uppercase(), None, None)); + if !self.match_token(TokenType::Comma) { + return Ok(properties); + } + } + } + + /// Whether the next tokens start `.`. + fn is_account_list(&self) -> bool { + matches!( + self.tokens + .get(self.current + 1) + .map(|token| token.token_type), + Some(TokenType::Dot) + ) + } + + /// Parse `. [ , ... ]`, refusing a reserved keyword as either part. + fn parse_account_list(&mut self) -> Result> { + let mut accounts: Vec = Vec::new(); + loop { + let name = self.parse_simple_qualified_name()?; + accounts.push(Expression::Table(Box::new(TableRef::new(name.name)))); + if !self.match_token(TokenType::Comma) { + return Ok(accounts); + } + } + } + + /// Parse an object name as an operand. + fn parse_group_name(&mut self) -> Result { + Ok(Expression::Table(Box::new(self.parse_object_name()?))) + } + + /// Parse ALTER ACCOUNT after its kind: the current account, or a named one. + fn parse_alter_account(&mut self) -> Result { + let named = !matches!( + self.text_at(0).as_deref(), + Some("SET" | "UNSET" | "ADD" | "REMOVE") + ); + let head = match named { + true => (self.parse_object_name()?, false), + false => (TableRef::new(""), false), + }; + let mut action = self.expect_alter_words(&[ + &["SET"], + &["UNSET"], + &["RENAME", "TO"], + &["DROP", "OLD", "ORGANIZATION", "URL"], + &["DROP", "OLD", "URL"], + &["ADD", "ORGANIZATION", "USER", "GROUP"], + &["REMOVE", "ORGANIZATION", "USER", "GROUP"], + ])?; + let set = matches!(action.as_str(), "SET"); + let parts: ActionParts = match action.as_str() { + "SET" | "UNSET" => { + if let Some(policy) = self.match_account_policy() { + action.push(' '); + action.push_str(&policy); + let operands = match set { + true => vec![self.parse_group_name()?], + false => vec![], + }; + let force = set && self.match_identifier("FORCE"); + if let Some(scope) = self.match_policy_scope() { + action.push_str(&format!(" FOR ALL {scope}")); + } + if set && (force || self.match_identifier("FORCE")) { + action.push_str(" FORCE"); + } + (operands, vec![]) + } else if self.check_identifier("TAG") && !self.check_next(TokenType::Eq) { + self.skip(); + (vec![], self.parse_group_tags(set)?) + } else if self.check_identifier("CONTACT") && !self.check_next(TokenType::Eq) { + self.skip(); + (vec![], self.parse_account_contacts(set)?) + } else if set { + (vec![], self.parse_integration_set()?) + } else { + (vec![], self.parse_group_unset_names()?) + } + } + "RENAME TO" => { + let name = self.parse_group_name()?; + let mut properties: Vec = Vec::new(); + if self.match_identifier("SAVE_OLD_URL") { + self.expect_token(TokenType::Eq)?; + properties.push(property( + "SAVE_OLD_URL", + None, + Some(self.parse_object_value()?), + )); + } + (vec![name], properties) + } + "ADD ORGANIZATION USER GROUP" | "REMOVE ORGANIZATION USER GROUP" => { + (vec![self.parse_group_name()?], vec![]) + } + _ => (vec![], vec![]), + }; + Ok(alter_object("ACCOUNT", head, action, parts)) + } + + /// Consume ` POLICY` for a policy an account sets, returning its words. + fn match_account_policy(&mut self) -> Option { + const POLICIES: &[&[&str]] = &[ + &["AUTHENTICATION", "POLICY"], + &["SESSION", "POLICY"], + &["PASSWORD", "POLICY"], + &["PACKAGES", "POLICY"], + &["DATA", "MOVEMENT", "POLICY"], + &["MULTI", "PARTY", "APPROVAL", "POLICY"], + &["FEATURE", "POLICY"], + &["MAINTENANCE", "POLICY"], + &["INGRESS", "POLICY"], + ]; + POLICIES + .iter() + .find(|words| self.match_text_seq(words)) + .map(|words| words.join(" ")) + } + + /// Consume `FOR ALL ` after an account policy, returning the scope's words. + fn match_policy_scope(&mut self) -> Option { + const SCOPES: &[&[&str]] = &[ + &["FOR", "ALL", "PERSON", "USERS"], + &["FOR", "ALL", "SERVICE", "USERS"], + &["FOR", "ALL", "APPLICATIONS"], + &["FOR", "ALL", "PERSONAL", "DATABASES"], + &["FOR", "ALL", "DATABASES"], + ]; + SCOPES + .iter() + .find(|words| self.match_text_seq(words)) + .map(|words| words[2..].join(" ")) + } + + /// Parse CONTACT purposes, with their contacts when `set`, comma-separated. + fn parse_account_contacts(&mut self, set: bool) -> Result> { + let mut properties: Vec = Vec::new(); + loop { + let purpose = self.parse_simple_name()?; + let value = match set { + true => { + self.expect_token(TokenType::Eq)?; + Some(self.parse_group_name()?) + } + false => None, + }; + properties.push(property("CONTACT", Some(purpose), value)); + if !self.match_token(TokenType::Comma) { + return Ok(properties); + } + } + } + + /// Consume `[ ] INTEGRATION` and return the kind, or None when the next words name another kind. + fn match_integration_kind(&mut self) -> Result> { + if self.match_identifier("INTEGRATION") { + return Ok(Some("INTEGRATION".to_owned())); + } + let kind: &[&str] = match self.text_at(0).as_deref() { + Some("EXTERNAL") if self.check_next_identifier("ACCESS") => &["EXTERNAL", "ACCESS"], + Some("API") => &["API"], + Some("CATALOG") => &["CATALOG"], + Some("NOTIFICATION") => &["NOTIFICATION"], + Some("SECURITY") => &["SECURITY"], + Some("STORAGE") if self.check_next_identifier("INTEGRATION") => &["STORAGE"], + _ => return Ok(None), + }; + self.current += kind.len(); + self.expect_alter_words(&[&["INTEGRATION"]])?; + Ok(Some(format!("{} INTEGRATION", kind.join(" ")))) + } + + /// Parse ALTER [ ] INTEGRATION after its kind. + fn parse_alter_integration(&mut self, kind: &str) -> Result { + let head = self.parse_alter_object_head()?; + let mut action = self.expect_alter_words(&[&["SET"], &["UNSET"], &["REFRESH"]])?; + let parts: ActionParts = match action.as_str() { + "SET" if self.check_identifier("TAG") && !self.check_next(TokenType::Eq) => { + self.skip(); + (vec![], self.parse_group_tags(true)?) + } + "SET" => (vec![], self.parse_integration_set()?), + "UNSET" => (vec![], self.parse_group_unset()?), + _ => { + while !self.is_at_end() && !self.check(TokenType::Semicolon) { + let key = self.expect_alter_words(&[ + &["OAUTH_CLIENT_SECRET_2"], + &["OAUTH_CLIENT_SECRET"], + &["SAML2_SNOWFLAKE_PRIVATE_KEY"], + &["METADATA_URL"], + ])?; + action.push(' '); + action.push_str(&key); + } + (vec![], vec![]) + } + }; + Ok(alter_object(kind, head, action, parts)) + } + + /// Parse an integration's SET: ` = ` properties, at least one, where a value may be a parenthesized list. + fn parse_integration_set(&mut self) -> Result> { + let mut properties: Vec = Vec::new(); + loop { + let name = self.parse_simple_name()?; + self.expect_token(TokenType::Eq)?; + let value = self.parse_integration_value()?; + properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + if self.is_at_end() || self.check(TokenType::Semicolon) { + return Ok(properties); + } + } + } + + /// Parse an integration property value: ALL, a literal or name, or `( ... )` of values or of ` = ` pairs, where a key may be any word, such as LOGIN_IDP_REDIRECT's DEFAULT. + fn parse_integration_value(&mut self) -> Result { + if self.text_at(0).as_deref() == Some("ALL") { + self.skip(); + return Ok(Expression::Table(Box::new(TableRef::new("ALL")))); + } + if !self.match_token(TokenType::LParen) { + return self.parse_object_value(); + } + let mut items: Vec = Vec::new(); + while !self.check(TokenType::RParen) { + if self.check_next(TokenType::Eq) { + let key = if self.check(TokenType::String) { + self.parse_string_value()? + } else if self.is_identifier_token() || self.check_keyword() { + let word = self.peek_text().to_ascii_uppercase(); + self.skip(); + Expression::Table(Box::new(TableRef::new(word))) + } else { + return Err(self.parse_error("Expected a key")); + }; + self.expect_token(TokenType::Eq)?; + let value = self.parse_object_value()?; + items.push(Expression::Tuple(Box::new(Tuple { + expressions: vec![key, value], + }))); + } else { + items.push(self.parse_object_value()?); + } + if !self.match_token(TokenType::Comma) { + break; + } + } + self.expect_token(TokenType::RParen)?; + Ok(Expression::Tuple(Box::new(Tuple { expressions: items }))) + } + + /// Parse ALTER CONNECTION after its kind. + fn parse_alter_connection(&mut self) -> Result { + let head = self.parse_alter_object_head()?; + let mut action = self.expect_alter_words(&[ + &["PRIMARY"], + &["ENABLE", "FAILOVER", "TO", "ACCOUNTS"], + &["DISABLE", "FAILOVER"], + &["SET", "COMMENT"], + &["UNSET", "COMMENT"], + ])?; + let parts: ActionParts = match action.as_str() { + "ENABLE FAILOVER TO ACCOUNTS" => (self.parse_account_list()?, vec![]), + "DISABLE FAILOVER" if self.match_text_seq(&["TO", "ACCOUNTS"]) => { + action.push_str(" TO ACCOUNTS"); + (self.parse_account_list()?, vec![]) + } + "SET COMMENT" => { + self.expect_token(TokenType::Eq)?; + ( + vec![], + vec![property("COMMENT", None, Some(self.parse_object_value()?))], + ) + } + _ => (vec![], vec![]), + }; + Ok(alter_object("CONNECTION", head, action, parts)) + } + + /// Parse ALTER ORGANIZATION after its first word: the organization itself, or its ACCOUNT, PROFILE, USER or USER GROUP. + fn parse_alter_organization(&mut self) -> Result { + if self.match_text_seq(&["USER", "GROUP"]) { + return self.parse_alter_organization_user_group(); + } + if self.match_identifier("USER") { + let head = self.parse_alter_object_head()?; + let action = self.expect_alter_words(&[&["SET"], &["UNSET"]])?; + let properties = match action.as_str() { + "SET" => self.parse_organization_set()?, + _ => self.parse_group_unset()?, + }; + return Ok(alter_object( + "ORGANIZATION USER", + head, + action, + (vec![], properties), + )); + } + if self.match_identifier("PROFILE") { + return self.parse_alter_organization_profile(); + } + let kind = match self.match_identifier("ACCOUNT") { + true => "ORGANIZATION ACCOUNT", + false => "ORGANIZATION", + }; + let head = (TableRef::new(""), false); + let mut action = self.expect_alter_words(&[&["SET"], &["UNSET"]])?; + let routing = + matches!(kind, "ORGANIZATION") && self.match_text_seq(&["EVENT", "ROUTING", "TABLE"]); + let parts: ActionParts = match (action.as_str(), routing) { + ("SET", true) => { + let table = self.parse_group_name()?; + self.expect_alter_words(&[&["FOR", "ALL", "APPLICATION", "LISTINGS"]])?; + action.push_str(" EVENT ROUTING TABLE FOR ALL APPLICATION LISTINGS"); + (vec![table], vec![]) + } + (_, true) => { + self.expect_alter_words(&[&["FOR", "ALL", "APPLICATION", "LISTINGS"]])?; + action.push_str(" EVENT ROUTING TABLE FOR ALL APPLICATION LISTINGS"); + (vec![], vec![]) + } + ("SET", false) => (vec![], self.parse_organization_set()?), + _ => (vec![], self.parse_group_unset_names()?), + }; + Ok(alter_object(kind, head, action, parts)) + } + + /// Parse ` = ` properties, at least one; a value may be a parenthesized list. + fn parse_organization_set(&mut self) -> Result> { + let mut properties: Vec = Vec::new(); + loop { + let name = self.parse_simple_name()?; + self.expect_token(TokenType::Eq)?; + let value = if self.match_token(TokenType::LParen) { + let mut items: Vec = Vec::new(); + while !self.check(TokenType::RParen) { + items.push(self.parse_object_value()?); + if !self.match_token(TokenType::Comma) { + break; + } + } + self.expect_token(TokenType::RParen)?; + Expression::Tuple(Box::new(Tuple { expressions: items })) + } else { + self.parse_object_value()? + }; + properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); + self.match_token(TokenType::Comma); + if self.is_at_end() || self.check(TokenType::Semicolon) { + return Ok(properties); + } + } + } + + /// Parse property names, comma-separated, with no TAG list. + fn parse_group_unset_names(&mut self) -> Result> { + let mut properties: Vec = Vec::new(); + loop { + let name = self.parse_simple_name()?; + properties.push(property(&name.name.to_ascii_uppercase(), None, None)); + if !self.match_token(TokenType::Comma) { + return Ok(properties); + } + } + } + + /// Parse ALTER ORGANIZATION USER GROUP after its kind. + fn parse_alter_organization_user_group(&mut self) -> Result { + let head = self.parse_alter_object_head()?; + let mut action = self.expect_alter_words(&[ + &["ADD", "ORGANIZATION", "USERS"], + &["REMOVE", "ORGANIZATION", "USERS"], + &["SET", "IS_GRANTABLE"], + &["SET", "VISIBILITY"], + ])?; + let parts: ActionParts = match action.as_str() { + "SET IS_GRANTABLE" => { + self.expect_token(TokenType::Eq)?; + (vec![self.parse_object_value()?], vec![]) + } + "SET VISIBILITY" => { + self.expect_token(TokenType::Eq)?; + let scope = + self.expect_alter_words(&[&["ALL"], &["ACCOUNTS"], &["REGION", "GROUPS"]])?; + action.push_str(&format!(" = {scope}")); + match scope.as_str() { + "ALL" => (vec![], vec![]), + "ACCOUNTS" => (self.parse_object_name_list()?, vec![]), + _ => { + let mut groups = vec![self.parse_string_value()?]; + while self.match_token(TokenType::Comma) { + groups.push(self.parse_string_value()?); + } + (groups, vec![]) + } + } + } + _ => (self.parse_object_name_list()?, vec![]), + }; + Ok(alter_object("ORGANIZATION USER GROUP", head, action, parts)) + } + + /// Parse ALTER ORGANIZATION PROFILE after its kind. + fn parse_alter_organization_profile(&mut self) -> Result { + let head = self.parse_alter_object_head()?; + let mut action = self.expect_alter_words(&[ + &["ABORT"], + &["COMMIT"], + &["PUBLISH"], + &["AS"], + &["RENAME", "TO"], + &["ADD", "LIVE", "VERSION"], + &["ADD", "VERSION"], + ])?; + let operands: Vec = match action.as_str() { + "AS" => vec![self.parse_string_value()?], + "RENAME TO" => vec![self.parse_group_name()?], + "ADD LIVE VERSION" | "ADD VERSION" => { + if self.match_keywords(&[TokenType::If, TokenType::Not, TokenType::Exists]) { + action.push_str(" IF NOT EXISTS"); + } + let mut operands: Vec = Vec::new(); + if !self.check(TokenType::From) { + operands.push(self.parse_group_name()?); + } + self.expect_token(TokenType::From)?; + if action.starts_with("ADD LIVE") { + self.expect_alter_words(&[&["LAST"]])?; + action.push_str(" FROM LAST"); + } else { + action.push_str(" FROM"); + operands.push(self.parse_location_value()?); + } + operands + } + _ => vec![], + }; + Ok(alter_object( + "ORGANIZATION PROFILE", + head, + action, + (operands, vec![]), + )) + } +} diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs index c9a8d6a2d..899218854 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/snowflake_alter_resources.rs @@ -12,6 +12,22 @@ use crate::tokens::types::Result; /// The object kinds this module parses, as the words that follow ALTER. const RESOURCE_KINDS: &[&[&str]] = &[ + &["MULTI", "PARTY", "APPROVAL", "POLICY"], + &["DATA", "MOVEMENT", "POLICY"], + &["AGGREGATION", "POLICY"], + &["AUTHENTICATION", "POLICY"], + &["BACKUP", "POLICY"], + &["FEATURE", "POLICY"], + &["INGRESS", "POLICY"], + &["JOIN", "POLICY"], + &["MAINTENANCE", "POLICY"], + &["MASKING", "POLICY"], + &["NETWORK", "POLICY"], + &["NETWORK", "RULE"], + &["PACKAGES", "POLICY"], + &["PASSWORD", "POLICY"], + &["SESSION", "POLICY"], + &["SNAPSHOT", "POLICY"], &["RESTRICTED", "SESSION", "SCOPE"], &["ARTIFACT", "REPOSITORY"], &["EXTERNAL", "VOLUME"], @@ -144,6 +160,16 @@ fn resource_actions(kind: &str) -> &'static [&'static [&'static str]] { &["UNSET"], ], "DATASET" => &[&["ADD", "VERSION"], &["DROP", "VERSION"]], + "FEATURE POLICY" | "INGRESS POLICY" | "MULTI PARTY APPROVAL POLICY" => { + &[&["AS"], &["RENAME", "TO"], &["SET"], &["UNSET"]] + } + "NETWORK POLICY" | "DATA MOVEMENT POLICY" => &[ + &["ADD"], + &["REMOVE"], + &["RENAME", "TO"], + &["SET"], + &["UNSET"], + ], "MATERIALIZED VIEW" => &[ &["CLUSTER", "BY"], &["DROP", "CLUSTERING", "KEY"], @@ -206,9 +232,12 @@ impl Parser { | ("EXTERNAL", "AGENT" | "TABLE" | "VOLUME" | "CONSUMER" | "ACCESS") => true, ("EVENT", "ROUTING") => matches!(self.text_at(2).as_deref(), Some("TABLE")), ("RESTRICTED", "SESSION") => matches!(self.text_at(2).as_deref(), Some("SCOPE")), + ("BACKUP", "SET") => return None, ( "COMPUTE" | "DBT" | "DCM" | "GIT" | "FILE" | "EXTERNAL" | "EVENT" | "ARTIFACT" - | "MATERIALIZED" | "RESTRICTED" | "VOLUME", + | "MATERIALIZED" | "RESTRICTED" | "VOLUME" | "AGGREGATION" | "AUTHENTICATION" + | "BACKUP" | "DATA" | "INGRESS" | "JOIN" | "MAINTENANCE" | "MASKING" | "MULTI" + | "NETWORK" | "PACKAGES" | "PASSWORD", _, ) => false, _ => return None, @@ -255,7 +284,18 @@ impl Parser { (_, "ADD FILES" | "REMOVE FILES") => (vec![self.parse_value_list()?], vec![]), (_, "ADD TARGETS" | "REMOVE TARGETS") => (vec![self.parse_resource_value()?], vec![]), ("STAGE" | "PIPE", "REFRESH") => (vec![], self.parse_resource_properties()?), - ("RESTRICTED SESSION SCOPE" | "RSS", "ADD" | "REMOVE" | "AS") => { + ("NETWORK POLICY" | "DATA MOVEMENT POLICY", "ADD" | "REMOVE") => { + (vec![], self.parse_resource_set()?) + } + ( + "RESTRICTED SESSION SCOPE" + | "RSS" + | "FEATURE POLICY" + | "INGRESS POLICY" + | "MULTI PARTY APPROVAL POLICY", + "AS", + ) + | ("RESTRICTED SESSION SCOPE" | "RSS", "ADD" | "REMOVE") => { (vec![self.parse_text_value()?], vec![]) } ("DATASET", "ADD VERSION") => return self.parse_dataset_version(action), @@ -475,7 +515,7 @@ impl Parser { } /// Parse a location: a stage path or a string. - fn parse_location_value(&mut self) -> Result { + pub(super) fn parse_location_value(&mut self) -> Result { if self.is_stage_path() { return self.parse_stage_path(); } @@ -566,7 +606,7 @@ impl Parser { Ok(properties) } - /// Parse zero or more TAG pairs, CONTACT pairs and ` = ` properties, optionally comma-separated. + /// Parse zero or more TAG pairs, CONTACT pairs and ` = ` properties, optionally comma-separated; a policy's `AS` body only follows another property. fn parse_resource_properties(&mut self) -> Result> { let mut properties: Vec = Vec::new(); while !self.is_at_end() && !self.check(TokenType::Semicolon) { @@ -587,10 +627,16 @@ impl Parser { break; } } + } else if !properties.is_empty() && self.match_token(TokenType::As) { + properties.push(property("AS", None, Some(self.parse_text_value()?))); } else { let name = self.parse_simple_name()?; - self.expect_token(TokenType::Eq)?; - let value = self.parse_resource_value()?; + let value = if self.match_token(TokenType::Arrow) { + self.parse_expression()? + } else { + self.expect_token(TokenType::Eq)?; + self.parse_resource_value()? + }; properties.push(property(&name.name.to_ascii_uppercase(), None, Some(value))); self.match_token(TokenType::Comma); } diff --git a/crates/sqlquack-syntax/tests/parser/test_snowflake_alter.rs b/crates/sqlquack-syntax/tests/parser/test_snowflake_alter.rs index 0a5a35b6d..fdb097fb1 100644 --- a/crates/sqlquack-syntax/tests/parser/test_snowflake_alter.rs +++ b/crates/sqlquack-syntax/tests/parser/test_snowflake_alter.rs @@ -257,6 +257,162 @@ fn given_a_snowflake_alter_of_an_object_kind_when_parsing_then_recorded_rejectio dialect: Dialect::Snowflake, expected_accepted: false, }, + AcceptanceTestCase { + description: "a policy body follows another SET property (recorded semantic error)", + sql: "ALTER INGRESS POLICY v1 SET COMMENT = 'm' AS $$m: 1 $$ ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a SET of a policy body alone is refused (recorded errno 1003)", + sql: "ALTER FEATURE POLICY v1 SET AS $$$$ ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a masking policy sets its body after an arrow (recorded semantic error)", + sql: "ALTER MASKING POLICY v1 SET BODY -> 'm' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a network policy adds to a rule list (recorded semantic error)", + sql: "ALTER NETWORK POLICY v1 ADD ALLOWED_NETWORK_RULE_LIST = ( 'm' ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a session policy is no ALTER SESSION (recorded semantic error)", + sql: "ALTER SESSION POLICY v1 SET ALLOWED_SECONDARY_ROLES = ( 'ALL' ) ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "ALTER SESSION still sets a parameter", + sql: "ALTER SESSION SET TIMEZONE = 'UTC' ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a snapshot policy unsets a property (recorded semantic error)", + sql: "ALTER SNAPSHOT POLICY v1 UNSET EXPIRE_AFTER_DAYS ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a snapshot policy takes no reserved keyword as its name (recorded errno 1003)", + sql: "ALTER SNAPSHOT POLICY SELECT UNSET COMMENT ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "MASKING without POLICY is no kind (recorded errno 1003)", + sql: "ALTER MASKING v1 RENAME TO v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "DATA POLICY lacks MOVEMENT (recorded errno 1003)", + sql: "ALTER DATA POLICY v1 RENAME TO v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a group sets comma lists, each ended by the next property (recorded semantic error)", + sql: "ALTER FAILOVER GROUP IF EXISTS v1 SET OBJECT_TYPES = DATABASES , SHARES ALLOWED_DATABASES = v1 , v1 ALLOWED_SHARES = v1 , v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "an integration type is a name of two words (recorded runtime error)", + sql: "ALTER FAILOVER GROUP v1 SET OBJECT_TYPES = INTEGRATIONS ALLOWED_INTEGRATION_TYPES = SECURITY INTEGRATIONS ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a group property value has no name of its own (recorded errno 1003)", + sql: "ALTER FAILOVER GROUP v1 SET OBJECT_TYPES = INTEGRATIONS = SECURITY INTEGRATIONS ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "allowed accounts are organization-qualified (recorded semantic error)", + sql: "ALTER FAILOVER GROUP IF EXISTS v1 ADD v1 . v1 , v1 . v1 TO ALLOWED_ACCOUNTS IGNORE EDITION CHECK ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a reserved keyword is no account name (recorded errno 1003)", + sql: "ALTER CONNECTION v1 ENABLE FAILOVER TO ACCOUNTS v1 . SELECT ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a replication group moves objects only to a REPLICATION GROUP (recorded errno 1003)", + sql: "ALTER REPLICATION GROUP v1 MOVE DATABASES v1 TO GROUP v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "GROUP alone is no kind (recorded errno 1003)", + sql: "ALTER GROUP v1 RENAME TO v1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "the organization sets its event routing table (recorded semantic error)", + sql: "ALTER ORGANIZATION SET EVENT ROUTING TABLE v1 FOR ALL APPLICATION LISTINGS ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "region groups are strings (recorded errno 1003)", + sql: "ALTER ORGANIZATION USER GROUP v1 SET VISIBILITY = REGION GROUPS m1 ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "an organization profile adds a live version (recorded semantic error)", + sql: "ALTER ORGANIZATION PROFILE v1 ADD LIVE VERSION IF NOT EXISTS v1 FROM LAST ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "ALL is an integration value (recorded semantic error)", + sql: "ALTER INTEGRATION v1 SET ALLOWED_AUTHENTICATION_SECRETS = all ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "a reserved keyword is no secret name (recorded errno 1003)", + sql: "ALTER INTEGRATION v1 SET ALLOWED_AUTHENTICATION_SECRETS = ( SELECT ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a webhook header needs a value (recorded errno 1003)", + sql: "ALTER INTEGRATION v1 SET WEBHOOK_HEADERS = ( 'm' = ) ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "EXTERNAL ACCESS without INTEGRATION is no kind (recorded errno 1003)", + sql: "ALTER EXTERNAL ACCESS v1 UNSET ALLOWED_NETWORK_RULES ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "an account redirect list keys on DEFAULT (recorded semantic error)", + sql: "ALTER ACCOUNT SET LOGIN_IDP_REDIRECT = ( DEFAULT = v1 , DEFAULT = v1 ) OBJECT_VISIBILITY = $$m: 1$$ ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, + AcceptanceTestCase { + description: "an account policy needs POLICY (recorded errno 1003)", + sql: "ALTER ACCOUNT SET FEATURE v1 FOR ALL APPLICATIONS ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, AcceptanceTestCase { description: "other dialects keep their ALTER fallback", sql: "ALTER v1 ADD RUN v1", From 9bebeadd6bbda3ce8d5f4fef4dc5b73102b6020c Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:05:49 +0000 Subject: [PATCH 2/2] fix(syntax): refuse a reserved keyword as a Snowflake DROP object name DROP of the Snowflake object kinds took any table reference as the name, so `DROP AGENT SELECT ;` parsed although Snowflake refuses it with errno 1003. The name now goes through the same object-name rule as ALTER: an identifier, never a string or an unquoted reserved keyword. A quoted `"SELECT"` is still a name. --- .../src/parser/_helpers/ddl/constraints.rs | 2 +- crates/sqlquack-syntax/tests/parser/test_fixes.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs index 9de0a7e70..e950c5ac3 100644 --- a/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs +++ b/crates/sqlquack-syntax/src/parser/_helpers/ddl/constraints.rs @@ -1458,7 +1458,7 @@ impl Parser { /// Parse a Snowflake DROP of `kind`, already consumed: the name and the options that kind takes. fn parse_drop_snowflake_object(&mut self, kind: String) -> Result { let if_exists = self.match_keywords(&[TokenType::If, TokenType::Exists]); - let name = self.parse_table_ref()?; + let name = self.parse_object_name()?; let (cascade, restrict, force) = match kind.as_str() { "APPLICATION" | "OPENFLOW RUNTIME" => { (self.match_token(TokenType::Cascade), false, false) diff --git a/crates/sqlquack-syntax/tests/parser/test_fixes.rs b/crates/sqlquack-syntax/tests/parser/test_fixes.rs index 45c8ea2c2..719046221 100644 --- a/crates/sqlquack-syntax/tests/parser/test_fixes.rs +++ b/crates/sqlquack-syntax/tests/parser/test_fixes.rs @@ -565,6 +565,18 @@ fn given_a_snowflake_drop_of_another_object_kind_when_parsing_then_it_takes_that dialect: Dialect::Snowflake, expected_accepted: false, }, + AcceptanceTestCase { + description: "an unquoted reserved keyword is no name (recorded errno 1003)", + sql: "DROP PIPE SELECT ;", + dialect: Dialect::Snowflake, + expected_accepted: false, + }, + AcceptanceTestCase { + description: "a quoted reserved keyword is a name (recorded semantic error)", + sql: "DROP PIPE \"SELECT\" ;", + dialect: Dialect::Snowflake, + expected_accepted: true, + }, AcceptanceTestCase { description: "IF without EXISTS (recorded errno 1003)", sql: "DROP AGENT IF v1 ;",