Skip to content

Commit e79119c

Browse files
LucaCappelletti94LucaCappelletti94
authored
Parse ALTER USER as a synonym for ALTER ROLE (apache#2374)
1 parent 021e97d commit e79119c

5 files changed

Lines changed: 128 additions & 46 deletions

File tree

‎src/dialect/mod.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,18 @@ pub trait Dialect: Debug + Any {
366366
false
367367
}
368368

369+
/// Returns true if the dialect treats `ALTER USER` as a synonym for `ALTER ROLE`.
370+
///
371+
/// In PostgreSQL, `ALTER USER` and `ALTER ROLE` are synonyms that accept the same
372+
/// option syntax, so `ALTER USER` is parsed into a [`Statement::AlterRole`].
373+
///
374+
/// <https://www.postgresql.org/docs/current/sql-alteruser.html>
375+
///
376+
/// [`Statement::AlterRole`]: crate::ast::Statement::AlterRole
377+
fn supports_alter_user_as_alter_role(&self) -> bool {
378+
false
379+
}
380+
369381
/// Returns true if the dialects supports `group sets, roll up, or cube` expressions.
370382
fn supports_group_by_expr(&self) -> bool {
371383
false

‎src/dialect/postgresql.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,10 @@ impl Dialect for PostgreSqlDialect {
161161
true
162162
}
163163

164+
fn supports_alter_user_as_alter_role(&self) -> bool {
165+
true
166+
}
167+
164168
fn prec_value(&self, prec: Precedence) -> u8 {
165169
match prec {
166170
Precedence::Period => PERIOD_PREC,

‎src/parser/mod.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11142,6 +11142,9 @@ impl<'a> Parser<'a> {
1114211142
Keyword::ROLE => self.parse_alter_role(),
1114311143
Keyword::POLICY => self.parse_alter_policy().map(Into::into),
1114411144
Keyword::CONNECTOR => self.parse_alter_connector(),
11145+
Keyword::USER if self.dialect.supports_alter_user_as_alter_role() => {
11146+
self.parse_alter_role()
11147+
}
1114511148
Keyword::USER => self.parse_alter_user().map(Into::into),
1114611149
// unreachable because expect_one_of_keywords used above
1114711150
unexpected_keyword => Err(ParserError::ParserError(

‎tests/sqlparser_common.rs‎

Lines changed: 59 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -18292,12 +18292,22 @@ fn key_value_option_statements_do_not_swallow_following_statement() {
1829218292
// terminator, otherwise any following statement fails to parse. This covers
1829318293
// every unparenthesized caller of `parse_key_value_options`: `CREATE USER`
1829418294
// and both `ALTER USER ... SET` forms.
18295+
18296+
// `CREATE USER` parses identically across all dialects.
18297+
let statements = all_dialects()
18298+
.parse_sql_statements("CREATE USER user1; SELECT 1")
18299+
.unwrap();
18300+
assert_eq!(statements.len(), 2);
18301+
18302+
// `ALTER USER ... SET` routes through the same key-value option list, but
18303+
// PostgreSQL parses `ALTER USER` as a synonym for `ALTER ROLE` (a different
18304+
// code path), so scope these to dialects that keep the Snowflake grammar.
18305+
let dialects = all_dialects_except(|d| d.supports_alter_user_as_alter_role());
1829518306
for sql in [
18296-
"CREATE USER user1; SELECT 1",
1829718307
"ALTER USER user1 SET x = 'y'; SELECT 1",
1829818308
"ALTER USER user1 SET TAG t = 'v'; SELECT 1",
1829918309
] {
18300-
let statements = all_dialects().parse_sql_statements(sql).unwrap();
18310+
let statements = dialects.parse_sql_statements(sql).unwrap();
1830118311
assert_eq!(statements.len(), 2, "{sql}");
1830218312
}
1830318313
}
@@ -18839,9 +18849,10 @@ fn parse_create_index_different_using_positions() {
1883918849

1884018850
#[test]
1884118851
fn test_parse_alter_user() {
18842-
verified_stmt("ALTER USER u1");
18843-
verified_stmt("ALTER USER IF EXISTS u1");
18844-
let stmt = verified_stmt("ALTER USER IF EXISTS u1 RENAME TO u2");
18852+
let dialects = all_dialects_except(|d| d.supports_alter_user_as_alter_role());
18853+
dialects.verified_stmt("ALTER USER u1");
18854+
dialects.verified_stmt("ALTER USER IF EXISTS u1");
18855+
let stmt = dialects.verified_stmt("ALTER USER IF EXISTS u1 RENAME TO u2");
1884518856
match stmt {
1884618857
Statement::AlterUser(alter) => {
1884718858
assert!(alter.if_exists);
@@ -18850,35 +18861,35 @@ fn test_parse_alter_user() {
1885018861
}
1885118862
_ => unreachable!(),
1885218863
}
18853-
verified_stmt("ALTER USER IF EXISTS u1 RESET PASSWORD");
18854-
verified_stmt("ALTER USER IF EXISTS u1 ABORT ALL QUERIES");
18855-
verified_stmt(
18864+
dialects.verified_stmt("ALTER USER IF EXISTS u1 RESET PASSWORD");
18865+
dialects.verified_stmt("ALTER USER IF EXISTS u1 ABORT ALL QUERIES");
18866+
dialects.verified_stmt(
1885618867
"ALTER USER IF EXISTS u1 ADD DELEGATED AUTHORIZATION OF ROLE r1 TO SECURITY INTEGRATION i1",
1885718868
);
18858-
verified_stmt("ALTER USER IF EXISTS u1 REMOVE DELEGATED AUTHORIZATION OF ROLE r1 FROM SECURITY INTEGRATION i1");
18859-
verified_stmt(
18869+
dialects.verified_stmt("ALTER USER IF EXISTS u1 REMOVE DELEGATED AUTHORIZATION OF ROLE r1 FROM SECURITY INTEGRATION i1");
18870+
dialects.verified_stmt(
1886018871
"ALTER USER IF EXISTS u1 REMOVE DELEGATED AUTHORIZATIONS FROM SECURITY INTEGRATION i1",
1886118872
);
18862-
verified_stmt("ALTER USER IF EXISTS u1 ENROLL MFA");
18863-
let stmt = verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD PASSKEY");
18873+
dialects.verified_stmt("ALTER USER IF EXISTS u1 ENROLL MFA");
18874+
let stmt = dialects.verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD PASSKEY");
1886418875
match stmt {
1886518876
Statement::AlterUser(alter) => {
1886618877
assert_eq!(alter.set_default_mfa_method, Some(MfaMethodKind::PassKey))
1886718878
}
1886818879
_ => unreachable!(),
1886918880
}
18870-
verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD TOTP");
18871-
verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD DUO");
18872-
let stmt = verified_stmt("ALTER USER u1 REMOVE MFA METHOD PASSKEY");
18881+
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD TOTP");
18882+
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD DUO");
18883+
let stmt = dialects.verified_stmt("ALTER USER u1 REMOVE MFA METHOD PASSKEY");
1887318884
match stmt {
1887418885
Statement::AlterUser(alter) => {
1887518886
assert_eq!(alter.remove_mfa_method, Some(MfaMethodKind::PassKey))
1887618887
}
1887718888
_ => unreachable!(),
1887818889
}
18879-
verified_stmt("ALTER USER u1 REMOVE MFA METHOD TOTP");
18880-
verified_stmt("ALTER USER u1 REMOVE MFA METHOD DUO");
18881-
let stmt = verified_stmt("ALTER USER u1 MODIFY MFA METHOD PASSKEY SET COMMENT 'abc'");
18890+
dialects.verified_stmt("ALTER USER u1 REMOVE MFA METHOD TOTP");
18891+
dialects.verified_stmt("ALTER USER u1 REMOVE MFA METHOD DUO");
18892+
let stmt = dialects.verified_stmt("ALTER USER u1 MODIFY MFA METHOD PASSKEY SET COMMENT 'abc'");
1888218893
match stmt {
1888318894
Statement::AlterUser(alter) => {
1888418895
assert_eq!(
@@ -18891,10 +18902,10 @@ fn test_parse_alter_user() {
1889118902
}
1889218903
_ => unreachable!(),
1889318904
}
18894-
verified_stmt("ALTER USER u1 ADD MFA METHOD OTP");
18895-
verified_stmt("ALTER USER u1 ADD MFA METHOD OTP COUNT = 8");
18905+
dialects.verified_stmt("ALTER USER u1 ADD MFA METHOD OTP");
18906+
dialects.verified_stmt("ALTER USER u1 ADD MFA METHOD OTP COUNT = 8");
1889618907

18897-
let stmt = verified_stmt("ALTER USER u1 SET AUTHENTICATION POLICY p1");
18908+
let stmt = dialects.verified_stmt("ALTER USER u1 SET AUTHENTICATION POLICY p1");
1889818909
match stmt {
1889918910
Statement::AlterUser(alter) => {
1890018911
assert_eq!(
@@ -18907,19 +18918,19 @@ fn test_parse_alter_user() {
1890718918
}
1890818919
_ => unreachable!(),
1890918920
}
18910-
verified_stmt("ALTER USER u1 SET PASSWORD POLICY p1");
18911-
verified_stmt("ALTER USER u1 SET SESSION POLICY p1");
18912-
let stmt = verified_stmt("ALTER USER u1 UNSET AUTHENTICATION POLICY");
18921+
dialects.verified_stmt("ALTER USER u1 SET PASSWORD POLICY p1");
18922+
dialects.verified_stmt("ALTER USER u1 SET SESSION POLICY p1");
18923+
let stmt = dialects.verified_stmt("ALTER USER u1 UNSET AUTHENTICATION POLICY");
1891318924
match stmt {
1891418925
Statement::AlterUser(alter) => {
1891518926
assert_eq!(alter.unset_policy, Some(UserPolicyKind::Authentication));
1891618927
}
1891718928
_ => unreachable!(),
1891818929
}
18919-
verified_stmt("ALTER USER u1 UNSET PASSWORD POLICY");
18920-
verified_stmt("ALTER USER u1 UNSET SESSION POLICY");
18930+
dialects.verified_stmt("ALTER USER u1 UNSET PASSWORD POLICY");
18931+
dialects.verified_stmt("ALTER USER u1 UNSET SESSION POLICY");
1892118932

18922-
let stmt = verified_stmt("ALTER USER u1 SET TAG k1='v1'");
18933+
let stmt = dialects.verified_stmt("ALTER USER u1 SET TAG k1='v1'");
1892318934
match stmt {
1892418935
Statement::AlterUser(alter) => {
1892518936
assert_eq!(
@@ -18934,23 +18945,25 @@ fn test_parse_alter_user() {
1893418945
}
1893518946
_ => unreachable!(),
1893618947
}
18937-
verified_stmt("ALTER USER u1 SET TAG k1='v1', k2='v2'");
18938-
let stmt = verified_stmt("ALTER USER u1 UNSET TAG k1");
18948+
dialects.verified_stmt("ALTER USER u1 SET TAG k1='v1', k2='v2'");
18949+
let stmt = dialects.verified_stmt("ALTER USER u1 UNSET TAG k1");
1893918950
match stmt {
1894018951
Statement::AlterUser(alter) => {
1894118952
assert_eq!(alter.unset_tag, vec!["k1".to_string()]);
1894218953
}
1894318954
_ => unreachable!(),
1894418955
}
18945-
verified_stmt("ALTER USER u1 UNSET TAG k1, k2, k3");
18956+
dialects.verified_stmt("ALTER USER u1 UNSET TAG k1, k2, k3");
1894618957

18947-
let dialects = all_dialects_where(|d| d.supports_boolean_literals());
18948-
dialects.one_statement_parses_to(
18958+
let bool_dialects = all_dialects_where(|d| {
18959+
d.supports_boolean_literals() && !d.supports_alter_user_as_alter_role()
18960+
});
18961+
bool_dialects.one_statement_parses_to(
1894918962
"ALTER USER u1 SET PASSWORD='secret', MUST_CHANGE_PASSWORD=TRUE, MINS_TO_UNLOCK=10",
1895018963
"ALTER USER u1 SET PASSWORD='secret', MUST_CHANGE_PASSWORD=true, MINS_TO_UNLOCK=10",
1895118964
);
1895218965

18953-
let stmt = dialects.verified_stmt(
18966+
let stmt = bool_dialects.verified_stmt(
1895418967
"ALTER USER u1 SET PASSWORD='secret', MUST_CHANGE_PASSWORD=true, MINS_TO_UNLOCK=10",
1895518968
);
1895618969
match stmt {
@@ -18985,16 +18998,16 @@ fn test_parse_alter_user() {
1898518998
_ => unreachable!(),
1898618999
}
1898719000

18988-
let stmt = verified_stmt("ALTER USER u1 UNSET PASSWORD");
19001+
let stmt = dialects.verified_stmt("ALTER USER u1 UNSET PASSWORD");
1898919002
match stmt {
1899019003
Statement::AlterUser(alter) => {
1899119004
assert_eq!(alter.unset_props, vec!["PASSWORD".to_string()]);
1899219005
}
1899319006
_ => unreachable!(),
1899419007
}
18995-
verified_stmt("ALTER USER u1 UNSET PASSWORD, MUST_CHANGE_PASSWORD, MINS_TO_UNLOCK");
19008+
dialects.verified_stmt("ALTER USER u1 UNSET PASSWORD, MUST_CHANGE_PASSWORD, MINS_TO_UNLOCK");
1899619009

18997-
let stmt = verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL')");
19010+
let stmt = dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL')");
1899819011
match stmt {
1899919012
Statement::AlterUser(alter) => {
1900019013
assert_eq!(
@@ -19010,11 +19023,11 @@ fn test_parse_alter_user() {
1901019023
}
1901119024
_ => unreachable!(),
1901219025
}
19013-
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=()");
19014-
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('R1', 'R2', 'R3')");
19015-
verified_stmt("ALTER USER u1 SET PASSWORD='secret', DEFAULT_SECONDARY_ROLES=('ALL')");
19016-
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret'");
19017-
let stmt = verified_stmt(
19026+
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=()");
19027+
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('R1', 'R2', 'R3')");
19028+
dialects.verified_stmt("ALTER USER u1 SET PASSWORD='secret', DEFAULT_SECONDARY_ROLES=('ALL')");
19029+
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret'");
19030+
let stmt = dialects.verified_stmt(
1901819031
"ALTER USER u1 SET WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')",
1901919032
);
1902019033
match stmt {
@@ -19048,13 +19061,13 @@ fn test_parse_alter_user() {
1904819061
}
1904919062
_ => unreachable!(),
1905019063
}
19051-
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret', WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')");
19064+
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret', WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')");
1905219065

19053-
verified_stmt("ALTER USER u1 PASSWORD 'AAA'");
19054-
verified_stmt("ALTER USER u1 ENCRYPTED PASSWORD 'AAA'");
19055-
verified_stmt("ALTER USER u1 PASSWORD NULL");
19066+
dialects.verified_stmt("ALTER USER u1 PASSWORD 'AAA'");
19067+
dialects.verified_stmt("ALTER USER u1 ENCRYPTED PASSWORD 'AAA'");
19068+
dialects.verified_stmt("ALTER USER u1 PASSWORD NULL");
1905619069

19057-
one_statement_parses_to(
19070+
dialects.one_statement_parses_to(
1905819071
"ALTER USER u1 WITH PASSWORD 'AAA'",
1905919072
"ALTER USER u1 PASSWORD 'AAA'",
1906019073
);

‎tests/sqlparser_postgres.rs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4641,6 +4641,56 @@ fn parse_alter_role() {
46414641
);
46424642
}
46434643

4644+
#[test]
4645+
fn parse_alter_user() {
4646+
// `ALTER USER` is a PostgreSQL synonym for `ALTER ROLE`, so it round-trips to `ALTER ROLE`.
4647+
let canonical = "ALTER ROLE old_name RENAME TO new_name";
4648+
assert_eq!(
4649+
pg().one_statement_parses_to("ALTER USER old_name RENAME TO new_name", canonical),
4650+
Statement::AlterRole {
4651+
name: Ident::new("old_name"),
4652+
operation: AlterRoleOperation::RenameRole {
4653+
role_name: Ident::new("new_name"),
4654+
},
4655+
}
4656+
);
4657+
4658+
let canonical = "ALTER ROLE bob WITH SUPERUSER PASSWORD 'x' CONNECTION LIMIT 5";
4659+
assert_eq!(
4660+
pg().one_statement_parses_to(
4661+
"ALTER USER bob WITH SUPERUSER PASSWORD 'x' CONNECTION LIMIT 5",
4662+
canonical
4663+
),
4664+
Statement::AlterRole {
4665+
name: Ident::new("bob"),
4666+
operation: AlterRoleOperation::WithOptions {
4667+
options: vec![
4668+
RoleOption::SuperUser(true),
4669+
RoleOption::Password(Password::Password(Expr::Value(
4670+
Value::SingleQuotedString("x".into()).with_empty_span()
4671+
))),
4672+
RoleOption::ConnectionLimit(Expr::value(number("5"))),
4673+
]
4674+
},
4675+
}
4676+
);
4677+
4678+
assert_eq!(
4679+
pg().one_statement_parses_to(
4680+
"ALTER USER bob SET search_path TO public",
4681+
"ALTER ROLE bob SET search_path TO public"
4682+
),
4683+
Statement::AlterRole {
4684+
name: Ident::new("bob"),
4685+
operation: AlterRoleOperation::Set {
4686+
config_name: ObjectName::from(vec![Ident::new("search_path")]),
4687+
config_value: SetConfigValue::Value(Expr::Identifier(Ident::new("public"))),
4688+
in_database: None,
4689+
},
4690+
}
4691+
);
4692+
}
4693+
46444694
#[test]
46454695
fn parse_delimited_identifiers() {
46464696
// check that quoted identifiers in any position remain quoted after serialization

0 commit comments

Comments
 (0)