Skip to content

Commit 3bcbf54

Browse files
hovaescoclaude
hovaesco
and
claude
authored
Task LAV-1715: ALTER SECURITY INTEGRATION: remaining SET properties + UNSET (apache#2226)
* Task LAV-1715: ALTER SECURITY INTEGRATION remaining SET properties + UNSET Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Task LAV-1715: verify OAUTH NETWORK_POLICY SET/UNSET against real Snowflake Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Task LAV-1715: fix CI — parse ALTER SECURITY INTEGRATION ... UNSET in the vendored parser The DDL rewriter matched on an `unset_options` field the vendored sqlparser's `Statement::AlterSecurityIntegration` never had, so the workspace didn't compile (E0026) and every CI job failed. Add `unset_options: Vec<Ident>` to the variant, parse `{ SET <kv> | UNSET <prop>[, ...] }` instead of requiring `SET`, and round-trip the `UNSET` form in `Display`. Covered by parser tests in vendor/sqlparser/tests/ per ADR 081 §6. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 24bc9dd commit 3bcbf54

3 files changed

Lines changed: 127 additions & 10 deletions

File tree

‎src/ast/mod.rs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5879,15 +5879,17 @@ pub enum Statement {
58795879
params: KeyValueOptions,
58805880
},
58815881
/// ```sql
5882-
/// ALTER SECURITY INTEGRATION [IF EXISTS] <name> SET ...
5882+
/// ALTER SECURITY INTEGRATION [IF EXISTS] <name> { SET ... | UNSET ... }
58835883
/// ```
58845884
AlterSecurityIntegration {
58855885
/// Security integration name.
58865886
name: ObjectName,
58875887
/// `IF EXISTS` flag.
58885888
if_exists: bool,
5889-
/// The `SET` options.
5889+
/// The `SET` options; empty for the `UNSET` form.
58905890
set_options: KeyValueOptions,
5891+
/// The property names of the `UNSET` form; empty for the `SET` form.
5892+
unset_options: Vec<Ident>,
58915893
},
58925894
/// ```sql
58935895
/// DROP SECURITY INTEGRATION [IF EXISTS] <name>
@@ -8777,14 +8779,20 @@ impl fmt::Display for Statement {
87778779
name,
87788780
if_exists,
87798781
set_options,
8782+
unset_options,
87808783
} => {
87818784
write!(
87828785
f,
8783-
"ALTER SECURITY INTEGRATION {if_exists}{name} SET",
8786+
"ALTER SECURITY INTEGRATION {if_exists}{name}",
87848787
if_exists = if *if_exists { "IF EXISTS " } else { "" },
87858788
)?;
8786-
if !set_options.options.is_empty() {
8787-
write!(f, " {set_options}")?;
8789+
if unset_options.is_empty() {
8790+
write!(f, " SET")?;
8791+
if !set_options.options.is_empty() {
8792+
write!(f, " {set_options}")?;
8793+
}
8794+
} else {
8795+
write!(f, " UNSET {}", display_comma_separated(unset_options))?;
87888796
}
87898797
Ok(())
87908798
}

‎src/dialect/snowflake.rs‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4715,19 +4715,37 @@ fn parse_create_security_integration(
47154715
})
47164716
}
47174717

4718-
/// Parse `ALTER SECURITY INTEGRATION [IF EXISTS] <name> SET <params>`.
4718+
/// Parse `ALTER SECURITY INTEGRATION [IF EXISTS] <name> { SET <params> | UNSET <props> }`.
47194719
///
4720-
/// Only the `SET` form is modeled; the `SET` options are captured generically
4721-
/// as key-value options.
4720+
/// The `SET` options are captured generically as key-value options; `UNSET`
4721+
/// takes a comma-separated list of property names. The parser stays agnostic to
4722+
/// which properties a given integration type accepts.
47224723
fn parse_alter_security_integration(parser: &mut Parser) -> Result<Statement, ParserError> {
47234724
let if_exists = parser.parse_keywords(&[Keyword::IF, Keyword::EXISTS]);
47244725
let name = parser.parse_object_name(false)?;
4725-
parser.expect_keyword(Keyword::SET)?;
4726-
let set_options = parser.parse_key_value_options(false, &[], false)?;
4726+
let empty_options = KeyValueOptions {
4727+
options: vec![],
4728+
delimiter: KeyValueOptionsDelimiter::Space,
4729+
};
4730+
let (set_options, unset_options) =
4731+
match parser.parse_one_of_keywords(&[Keyword::SET, Keyword::UNSET]) {
4732+
Some(Keyword::SET) => (parser.parse_key_value_options(false, &[], false)?, vec![]),
4733+
Some(Keyword::UNSET) => (
4734+
empty_options,
4735+
parser.parse_comma_separated(Parser::parse_identifier)?,
4736+
),
4737+
_ => {
4738+
return parser.expected(
4739+
"SET or UNSET after ALTER SECURITY INTEGRATION <name>",
4740+
parser.peek_token(),
4741+
)
4742+
}
4743+
};
47274744
Ok(Statement::AlterSecurityIntegration {
47284745
name,
47294746
if_exists,
47304747
set_options,
4748+
unset_options,
47314749
})
47324750
}
47334751

‎tests/sqlparser_snowflake.rs‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9953,3 +9953,94 @@ fn parse_sf_alter_role_set_tag_still_intercepted() {
99539953
_ => unreachable!(),
99549954
}
99559955
}
9956+
9957+
#[test]
9958+
fn parse_sf_alter_security_integration_set() {
9959+
match snowflake().verified_stmt("ALTER SECURITY INTEGRATION i SET ENABLED=false") {
9960+
Statement::AlterSecurityIntegration {
9961+
name,
9962+
if_exists,
9963+
set_options,
9964+
unset_options,
9965+
} => {
9966+
assert_eq!("i", name.to_string());
9967+
assert!(!if_exists);
9968+
assert_eq!(1, set_options.options.len());
9969+
assert_eq!("ENABLED", set_options.options[0].option_name);
9970+
assert!(unset_options.is_empty());
9971+
}
9972+
_ => unreachable!(),
9973+
}
9974+
}
9975+
9976+
#[test]
9977+
fn parse_sf_alter_security_integration_unset() {
9978+
match snowflake().verified_stmt("ALTER SECURITY INTEGRATION i UNSET NETWORK_POLICY") {
9979+
Statement::AlterSecurityIntegration {
9980+
name,
9981+
if_exists,
9982+
set_options,
9983+
unset_options,
9984+
} => {
9985+
assert_eq!("i", name.to_string());
9986+
assert!(!if_exists);
9987+
assert!(set_options.options.is_empty());
9988+
assert_eq!(
9989+
vec![Ident::new("NETWORK_POLICY")],
9990+
unset_options
9991+
.iter()
9992+
.map(|i| Ident::new(i.value.clone()))
9993+
.collect::<Vec<_>>()
9994+
);
9995+
}
9996+
_ => unreachable!(),
9997+
}
9998+
}
9999+
10000+
#[test]
10001+
fn parse_sf_alter_security_integration_unset_multiple_with_if_exists() {
10002+
// A comma-separated UNSET list; COMMENT is a keyword but is still accepted
10003+
// as a bare property name.
10004+
match snowflake().verified_stmt("ALTER SECURITY INTEGRATION IF EXISTS i UNSET ENABLED, COMMENT")
10005+
{
10006+
Statement::AlterSecurityIntegration {
10007+
if_exists,
10008+
set_options,
10009+
unset_options,
10010+
..
10011+
} => {
10012+
assert!(if_exists);
10013+
assert!(set_options.options.is_empty());
10014+
assert_eq!(
10015+
vec!["ENABLED".to_string(), "COMMENT".to_string()],
10016+
unset_options
10017+
.iter()
10018+
.map(|i| i.value.clone())
10019+
.collect::<Vec<_>>()
10020+
);
10021+
}
10022+
_ => unreachable!(),
10023+
}
10024+
}
10025+
10026+
#[test]
10027+
fn parse_sf_alter_security_integration_requires_set_or_unset() {
10028+
assert_eq!(
10029+
snowflake()
10030+
.parse_sql_statements("ALTER SECURITY INTEGRATION i")
10031+
.unwrap_err()
10032+
.to_string(),
10033+
"sql parser error: Expected: SET or UNSET after ALTER SECURITY INTEGRATION <name>, found: EOF"
10034+
);
10035+
}
10036+
10037+
#[test]
10038+
fn parse_sf_alter_security_integration_unset_requires_a_property() {
10039+
assert_eq!(
10040+
snowflake()
10041+
.parse_sql_statements("ALTER SECURITY INTEGRATION i UNSET")
10042+
.unwrap_err()
10043+
.to_string(),
10044+
"sql parser error: Expected: identifier, found: EOF"
10045+
);
10046+
}

0 commit comments

Comments
 (0)