Skip to content

Parse ALTER USER as a synonym for ALTER ROLE #2374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LucaCappelletti94 wants to merge 1 commit into
base: main
Choose a base branch
from
+116 −44
Open

Parse ALTER USER as a synonym for ALTER ROLE #2374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/dialect/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,18 @@ pub trait Dialect: Debug + Any {
false
}

/// Returns true if the dialect treats `ALTER USER` as a synonym for `ALTER ROLE`.
///
/// In PostgreSQL, `ALTER USER` and `ALTER ROLE` are synonyms that accept the same
/// option syntax, so `ALTER USER` is parsed into a [`Statement::AlterRole`].
///
/// <https://www.postgresql.org/docs/current/sql-alteruser.html>
///
/// [`Statement::AlterRole`]: crate::ast::Statement::AlterRole
fn supports_alter_user_as_alter_role(&self) -> bool {
false
}

/// Returns true if the dialects supports `group sets, roll up, or cube` expressions.
fn supports_group_by_expr(&self) -> bool {
false
Expand Down
4 changes: 4 additions & 0 deletions src/dialect/postgresql.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ impl Dialect for PostgreSqlDialect {
true
}

fn supports_alter_user_as_alter_role(&self) -> bool {
true
}

fn prec_value(&self, prec: Precedence) -> u8 {
match prec {
Precedence::Period => PERIOD_PREC,
Expand Down
3 changes: 3 additions & 0 deletions src/parser/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10955,6 +10955,9 @@ impl<'a> Parser<'a> {
Keyword::ROLE => self.parse_alter_role(),
Keyword::POLICY => self.parse_alter_policy().map(Into::into),
Keyword::CONNECTOR => self.parse_alter_connector(),
Keyword::USER if self.dialect.supports_alter_user_as_alter_role() => {
self.parse_alter_role()
}
Keyword::USER => self.parse_alter_user().map(Into::into),
// unreachable because expect_one_of_keywords used above
unexpected_keyword => Err(ParserError::ParserError(
Expand Down
91 changes: 47 additions & 44 deletions tests/sqlparser_common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18479,9 +18479,10 @@ fn parse_create_index_different_using_positions() {

#[test]
fn test_parse_alter_user() {
verified_stmt("ALTER USER u1");
verified_stmt("ALTER USER IF EXISTS u1");
let stmt = verified_stmt("ALTER USER IF EXISTS u1 RENAME TO u2");
let dialects = all_dialects_except(|d| d.supports_alter_user_as_alter_role());
dialects.verified_stmt("ALTER USER u1");
dialects.verified_stmt("ALTER USER IF EXISTS u1");
let stmt = dialects.verified_stmt("ALTER USER IF EXISTS u1 RENAME TO u2");
match stmt {
Statement::AlterUser(alter) => {
assert!(alter.if_exists);
Expand All @@ -18490,35 +18491,35 @@ fn test_parse_alter_user() {
}
_ => unreachable!(),
}
verified_stmt("ALTER USER IF EXISTS u1 RESET PASSWORD");
verified_stmt("ALTER USER IF EXISTS u1 ABORT ALL QUERIES");
verified_stmt(
dialects.verified_stmt("ALTER USER IF EXISTS u1 RESET PASSWORD");
dialects.verified_stmt("ALTER USER IF EXISTS u1 ABORT ALL QUERIES");
dialects.verified_stmt(
"ALTER USER IF EXISTS u1 ADD DELEGATED AUTHORIZATION OF ROLE r1 TO SECURITY INTEGRATION i1",
);
verified_stmt("ALTER USER IF EXISTS u1 REMOVE DELEGATED AUTHORIZATION OF ROLE r1 FROM SECURITY INTEGRATION i1");
verified_stmt(
dialects.verified_stmt("ALTER USER IF EXISTS u1 REMOVE DELEGATED AUTHORIZATION OF ROLE r1 FROM SECURITY INTEGRATION i1");
dialects.verified_stmt(
"ALTER USER IF EXISTS u1 REMOVE DELEGATED AUTHORIZATIONS FROM SECURITY INTEGRATION i1",
);
verified_stmt("ALTER USER IF EXISTS u1 ENROLL MFA");
let stmt = verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD PASSKEY");
dialects.verified_stmt("ALTER USER IF EXISTS u1 ENROLL MFA");
let stmt = dialects.verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD PASSKEY");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(alter.set_default_mfa_method, Some(MfaMethodKind::PassKey))
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD TOTP");
verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD DUO");
let stmt = verified_stmt("ALTER USER u1 REMOVE MFA METHOD PASSKEY");
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD TOTP");
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_MFA_METHOD DUO");
let stmt = dialects.verified_stmt("ALTER USER u1 REMOVE MFA METHOD PASSKEY");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(alter.remove_mfa_method, Some(MfaMethodKind::PassKey))
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 REMOVE MFA METHOD TOTP");
verified_stmt("ALTER USER u1 REMOVE MFA METHOD DUO");
let stmt = verified_stmt("ALTER USER u1 MODIFY MFA METHOD PASSKEY SET COMMENT 'abc'");
dialects.verified_stmt("ALTER USER u1 REMOVE MFA METHOD TOTP");
dialects.verified_stmt("ALTER USER u1 REMOVE MFA METHOD DUO");
let stmt = dialects.verified_stmt("ALTER USER u1 MODIFY MFA METHOD PASSKEY SET COMMENT 'abc'");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(
Expand All @@ -18531,10 +18532,10 @@ fn test_parse_alter_user() {
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 ADD MFA METHOD OTP");
verified_stmt("ALTER USER u1 ADD MFA METHOD OTP COUNT = 8");
dialects.verified_stmt("ALTER USER u1 ADD MFA METHOD OTP");
dialects.verified_stmt("ALTER USER u1 ADD MFA METHOD OTP COUNT = 8");

let stmt = verified_stmt("ALTER USER u1 SET AUTHENTICATION POLICY p1");
let stmt = dialects.verified_stmt("ALTER USER u1 SET AUTHENTICATION POLICY p1");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(
Expand All @@ -18547,19 +18548,19 @@ fn test_parse_alter_user() {
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 SET PASSWORD POLICY p1");
verified_stmt("ALTER USER u1 SET SESSION POLICY p1");
let stmt = verified_stmt("ALTER USER u1 UNSET AUTHENTICATION POLICY");
dialects.verified_stmt("ALTER USER u1 SET PASSWORD POLICY p1");
dialects.verified_stmt("ALTER USER u1 SET SESSION POLICY p1");
let stmt = dialects.verified_stmt("ALTER USER u1 UNSET AUTHENTICATION POLICY");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(alter.unset_policy, Some(UserPolicyKind::Authentication));
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 UNSET PASSWORD POLICY");
verified_stmt("ALTER USER u1 UNSET SESSION POLICY");
dialects.verified_stmt("ALTER USER u1 UNSET PASSWORD POLICY");
dialects.verified_stmt("ALTER USER u1 UNSET SESSION POLICY");

let stmt = verified_stmt("ALTER USER u1 SET TAG k1='v1'");
let stmt = dialects.verified_stmt("ALTER USER u1 SET TAG k1='v1'");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(
Expand All @@ -18574,23 +18575,25 @@ fn test_parse_alter_user() {
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 SET TAG k1='v1', k2='v2'");
let stmt = verified_stmt("ALTER USER u1 UNSET TAG k1");
dialects.verified_stmt("ALTER USER u1 SET TAG k1='v1', k2='v2'");
let stmt = dialects.verified_stmt("ALTER USER u1 UNSET TAG k1");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(alter.unset_tag, vec!["k1".to_string()]);
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 UNSET TAG k1, k2, k3");
dialects.verified_stmt("ALTER USER u1 UNSET TAG k1, k2, k3");

let dialects = all_dialects_where(|d| d.supports_boolean_literals());
dialects.one_statement_parses_to(
let bool_dialects = all_dialects_where(|d| {
d.supports_boolean_literals() && !d.supports_alter_user_as_alter_role()
});
bool_dialects.one_statement_parses_to(
"ALTER USER u1 SET PASSWORD='secret', MUST_CHANGE_PASSWORD=TRUE, MINS_TO_UNLOCK=10",
"ALTER USER u1 SET PASSWORD='secret', MUST_CHANGE_PASSWORD=true, MINS_TO_UNLOCK=10",
);

let stmt = dialects.verified_stmt(
let stmt = bool_dialects.verified_stmt(
"ALTER USER u1 SET PASSWORD='secret', MUST_CHANGE_PASSWORD=true, MINS_TO_UNLOCK=10",
);
match stmt {
Expand Down Expand Up @@ -18625,16 +18628,16 @@ fn test_parse_alter_user() {
_ => unreachable!(),
}

let stmt = verified_stmt("ALTER USER u1 UNSET PASSWORD");
let stmt = dialects.verified_stmt("ALTER USER u1 UNSET PASSWORD");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(alter.unset_props, vec!["PASSWORD".to_string()]);
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 UNSET PASSWORD, MUST_CHANGE_PASSWORD, MINS_TO_UNLOCK");
dialects.verified_stmt("ALTER USER u1 UNSET PASSWORD, MUST_CHANGE_PASSWORD, MINS_TO_UNLOCK");

let stmt = verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL')");
let stmt = dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL')");
match stmt {
Statement::AlterUser(alter) => {
assert_eq!(
Expand All @@ -18650,11 +18653,11 @@ fn test_parse_alter_user() {
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=()");
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('R1', 'R2', 'R3')");
verified_stmt("ALTER USER u1 SET PASSWORD='secret', DEFAULT_SECONDARY_ROLES=('ALL')");
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret'");
let stmt = verified_stmt(
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=()");
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('R1', 'R2', 'R3')");
dialects.verified_stmt("ALTER USER u1 SET PASSWORD='secret', DEFAULT_SECONDARY_ROLES=('ALL')");
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret'");
let stmt = dialects.verified_stmt(
"ALTER USER u1 SET WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')",
);
match stmt {
Expand Down Expand Up @@ -18688,13 +18691,13 @@ fn test_parse_alter_user() {
}
_ => unreachable!(),
}
verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret', WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')");
dialects.verified_stmt("ALTER USER u1 SET DEFAULT_SECONDARY_ROLES=('ALL'), PASSWORD='secret', WORKLOAD_IDENTITY=(TYPE=AWS, ARN='arn:aws:iam::123456789:r1/')");

verified_stmt("ALTER USER u1 PASSWORD 'AAA'");
verified_stmt("ALTER USER u1 ENCRYPTED PASSWORD 'AAA'");
verified_stmt("ALTER USER u1 PASSWORD NULL");
dialects.verified_stmt("ALTER USER u1 PASSWORD 'AAA'");
dialects.verified_stmt("ALTER USER u1 ENCRYPTED PASSWORD 'AAA'");
dialects.verified_stmt("ALTER USER u1 PASSWORD NULL");

one_statement_parses_to(
dialects.one_statement_parses_to(
"ALTER USER u1 WITH PASSWORD 'AAA'",
"ALTER USER u1 PASSWORD 'AAA'",
);
Expand Down
50 changes: 50 additions & 0 deletions tests/sqlparser_postgres.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4546,6 +4546,56 @@ fn parse_alter_role() {
);
}

#[test]
fn parse_alter_user() {
// `ALTER USER` is a PostgreSQL synonym for `ALTER ROLE`, so it round-trips to `ALTER ROLE`.
let canonical = "ALTER ROLE old_name RENAME TO new_name";
assert_eq!(
pg().one_statement_parses_to("ALTER USER old_name RENAME TO new_name", canonical),
Statement::AlterRole {
name: Ident::new("old_name"),
operation: AlterRoleOperation::RenameRole {
role_name: Ident::new("new_name"),
},
}
);

let canonical = "ALTER ROLE bob WITH SUPERUSER PASSWORD 'x' CONNECTION LIMIT 5";
assert_eq!(
pg().one_statement_parses_to(
"ALTER USER bob WITH SUPERUSER PASSWORD 'x' CONNECTION LIMIT 5",
canonical
),
Statement::AlterRole {
name: Ident::new("bob"),
operation: AlterRoleOperation::WithOptions {
options: vec![
RoleOption::SuperUser(true),
RoleOption::Password(Password::Password(Expr::Value(
Value::SingleQuotedString("x".into()).with_empty_span()
))),
RoleOption::ConnectionLimit(Expr::value(number("5"))),
]
},
}
);

assert_eq!(
pg().one_statement_parses_to(
"ALTER USER bob SET search_path TO public",
"ALTER ROLE bob SET search_path TO public"
),
Statement::AlterRole {
name: Ident::new("bob"),
operation: AlterRoleOperation::Set {
config_name: ObjectName::from(vec![Ident::new("search_path")]),
config_value: SetConfigValue::Value(Expr::Identifier(Ident::new("public"))),
in_database: None,
},
}
);
}

#[test]
fn parse_delimited_identifiers() {
// check that quoted identifiers in any position remain quoted after serialization
Expand Down
Loading