Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions docs/dev/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
- [Relational Table](./design/relational-table.md)
- [Multiple Object Storage Backends](./design/multi-object-store.md)
- [Meta Service](./design/meta-service.md)
- [RBAC Parser Surface](./design/rbac-parser-surface.md)
- [Data Model and Encoding](./design/data-model-and-encoding.md)
- [Batch Local Execution Mode](./design/batch-local-execution-mode.md)
- [Consistent Hash](./design/consistent-hash.md)
Expand Down
54 changes: 54 additions & 0 deletions docs/dev/src/design/rbac-parser-surface.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# PostgreSQL-style RBAC parser surface

This note documents the staged parser and dispatch surface that was added for
PostgreSQL-style RBAC compatibility work.

## Scope

The SQL parser and frontend statement dispatcher now recognize these statements:

- `CREATE ROLE`
- `ALTER ROLE`
- role-membership `GRANT ... TO ... [WITH ADMIN OPTION]`
- role-membership `REVOKE [ADMIN OPTION FOR] ... FROM ...`
- `SET ROLE`
- `RESET ROLE`

The parser also accepts the PostgreSQL-style role options currently needed by the
test matrix for this lane, including:

- `CREATEDB` / `NOCREATEDB`
- `CREATEROLE` / `NOCREATEROLE`
- `INHERIT` / `NOINHERIT`
- `LOGIN` / `NOLOGIN`
- legacy RisingWave user aliases that already existed

## Current implementation boundary

This lane intentionally lands parser and dispatcher support before the meta and
session lanes are finished.

That means some statements are **accepted and dispatched** but still terminate in
explicit `not implemented` frontend handlers until the downstream lanes land:

- membership grant/revoke execution
- `SET ROLE`
- `RESET ROLE`

This staged boundary is deliberate so that:

1. parser and AST contracts stabilize early,
2. regression tests can be written against a fixed SQL surface, and
3. meta/session work can plug into the already-routed statement variants.

## Verification coverage

The worker verification for this lane covers both unit-style parser assertions and
parser-test YAML contracts:

- `sqlparser_common` role-focused tests
- `role.yaml`
- `drop.yaml`

These suites are the source of truth for the accepted SQL spellings and formatter
output of the parser surface described above.
Comment thread
tabVersion marked this conversation as resolved.
27 changes: 27 additions & 0 deletions src/frontend/src/handler/alter_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,15 @@ fn alter_prost_user_info(
user_info.is_admin = false;
update_fields.insert(UpdateField::Admin);
}
UserOption::CreateRole
| UserOption::NoCreateRole
| UserOption::Inherit
| UserOption::NoInherit => {
return Err(ErrorCode::InvalidParameterValue(
"role options are not supported yet".to_owned(),
)
.into());
}
Comment thread
tabVersion marked this conversation as resolved.
UserOption::EncryptedPassword(p) => {
if !p.0.is_empty() {
user_info.auth_info = encrypted_password(&user_info.name, &p.0);
Expand Down Expand Up @@ -299,6 +308,24 @@ mod tests {
);
}

#[tokio::test]
async fn test_alter_user_rejects_role_options() {
let frontend = LocalFrontend::new(Default::default()).await;
frontend
.run_sql("CREATE USER role_option_user")
.await
.unwrap();

for option in ["CREATEROLE", "NOCREATEROLE", "INHERIT", "NOINHERIT"] {
let err = frontend
.run_sql(&format!("ALTER USER role_option_user WITH {option}"))
.await
.unwrap_err()
.to_string();
assert!(err.contains("role options are not supported yet"), "{err}");
}
}

#[tokio::test]
async fn test_alter_admin_user() {
let frontend = LocalFrontend::new(Default::default()).await;
Expand Down
26 changes: 26 additions & 0 deletions src/frontend/src/handler/create_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ pub async fn handle_create_user(
UserOption::NoLogin => user_info.can_login = false,
UserOption::Admin => user_info.is_admin = true,
UserOption::NoAdmin => user_info.is_admin = false,
UserOption::CreateRole
| UserOption::NoCreateRole
| UserOption::Inherit
| UserOption::NoInherit => {
return Err(ErrorCode::InvalidParameterValue(
"role options are not supported yet".to_owned(),
)
.into());
}
Comment thread
tabVersion marked this conversation as resolved.
UserOption::EncryptedPassword(password) => {
if !password.0.is_empty() {
user_info.auth_info = encrypted_password(&user_info.name, &password.0);
Expand Down Expand Up @@ -235,6 +244,23 @@ mod tests {
);
}

#[tokio::test]
async fn test_create_user_rejects_role_options() {
let frontend = LocalFrontend::new(Default::default()).await;

for (idx, option) in ["CREATEROLE", "NOCREATEROLE", "INHERIT", "NOINHERIT"]
.into_iter()
.enumerate()
{
let err = frontend
.run_sql(&format!("CREATE USER role_option_user_{idx} WITH {option}"))
.await
.unwrap_err()
.to_string();
assert!(err.contains("role options are not supported yet"), "{err}");
}
}

#[tokio::test]
async fn test_create_admin_user() {
let frontend = LocalFrontend::new(Default::default()).await;
Expand Down
5 changes: 4 additions & 1 deletion src/frontend/src/handler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ pub async fn handle(
| ObjectType::Schema
| ObjectType::Connection
| ObjectType::Secret => true,
ObjectType::Database | ObjectType::User => {
ObjectType::Database | ObjectType::User | ObjectType::Role => {
bail_not_implemented!("DROP CASCADE");
}
}
Expand Down Expand Up @@ -580,6 +580,9 @@ pub async fn handle(
ObjectType::User => {
drop_user::handle_drop_user(handler_args, object_name, if_exists).await
}
ObjectType::Role => {
bail_not_implemented!("DROP ROLE")
}
ObjectType::View => {
drop_view::handle_drop_view(handler_args, object_name, if_exists, cascade).await
}
Expand Down
88 changes: 87 additions & 1 deletion src/sqlparser/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1633,6 +1633,13 @@ pub enum Statement {
with_grant_option: bool,
granted_by: Option<Ident>,
},
/// GRANT roles TO grantees
GrantRole {
roles: Vec<Ident>,
grantees: Vec<Ident>,
role_options: Vec<RoleOptionSpec>,
granted_by: Option<Ident>,
},
/// REVOKE privileges ON objects FROM grantees
Revoke {
privileges: Privileges,
Expand All @@ -1642,6 +1649,14 @@ pub enum Statement {
revoke_grant_option: bool,
cascade: bool,
},
/// REVOKE roles FROM grantees
RevokeRole {
roles: Vec<Ident>,
grantees: Vec<Ident>,
revoke_role_option: Option<RoleOptionKind>,
granted_by: Option<Ident>,
cascade: bool,
},
/// `DEALLOCATE [ PREPARE ] { name | ALL }`
///
/// Note: this is a PostgreSQL-specific statement.
Expand Down Expand Up @@ -1683,8 +1698,19 @@ pub enum Statement {
},
/// CREATE USER
CreateUser(CreateUserStatement),
/// CREATE ROLE
CreateRole(CreateUserStatement),
/// ALTER USER
AlterUser(AlterUserStatement),
/// ALTER ROLE
AlterRole(AlterUserStatement),
/// `SET [ SESSION | LOCAL ] ROLE <role_name | NONE>`
SetRole {
context_modifier: Option<RoleContextModifier>,
role_name: SetRoleSpec,
},
/// `RESET ROLE`
ResetRole,
/// ALTER SYSTEM SET configuration_parameter { TO | = } { value | 'value' | DEFAULT }
AlterSystem {
param: Ident,
Expand Down Expand Up @@ -2377,6 +2403,22 @@ impl Statement {
}
Ok(())
}
Statement::GrantRole {
roles,
grantees,
role_options,
granted_by,
} => {
write!(f, "GRANT {} ", display_comma_separated(roles))?;
write!(f, "TO {}", display_comma_separated(grantees))?;
if !role_options.is_empty() {
write!(f, " WITH {}", display_comma_separated(role_options))?;
}
if let Some(grantor) = granted_by {
write!(f, " GRANTED BY {}", grantor)?;
}
Ok(())
}
Statement::Revoke {
privileges,
objects,
Expand All @@ -2403,6 +2445,28 @@ impl Statement {
write!(f, " {}", if *cascade { "CASCADE" } else { "RESTRICT" })?;
Ok(())
}
Statement::RevokeRole {
roles,
grantees,
revoke_role_option,
granted_by,
cascade,
} => {
write!(
f,
"REVOKE {}{} FROM {}",
revoke_role_option
.as_ref()
.map(|opt| format!("{opt} OPTION FOR "))
.unwrap_or_default(),
display_comma_separated(roles),
display_comma_separated(grantees),
)?;
if let Some(grantor) = granted_by {
write!(f, " GRANTED BY {}", grantor)?;
}
write!(f, " {}", if *cascade { "CASCADE" } else { "RESTRICT" })
}
Statement::Deallocate { name, prepare } => {
if let Some(name) = name {
write!(
Expand Down Expand Up @@ -2453,9 +2517,26 @@ impl Statement {
Statement::CreateUser(statement) => {
write!(f, "CREATE USER {}", statement)
}
Statement::CreateRole(statement) => {
write!(f, "CREATE ROLE {}", statement)
}
Statement::AlterUser(statement) => {
write!(f, "ALTER USER {}", statement)
}
Statement::AlterRole(statement) => {
write!(f, "ALTER ROLE {}", statement)
}
Statement::SetRole {
context_modifier,
role_name,
} => {
write!(f, "SET ")?;
if let Some(modifier) = context_modifier {
write!(f, "{} ", modifier)?;
}
write!(f, "ROLE {}", role_name)
}
Statement::ResetRole => write!(f, "RESET ROLE"),
Statement::AlterSystem { param, value } => {
f.write_str("ALTER SYSTEM SET ")?;
write!(f, "{param} = {value}",)
Expand Down Expand Up @@ -2546,6 +2627,7 @@ impl Statement {
| Statement::CreateConnection { .. }
| Statement::CreateSecret { .. }
| Statement::CreateUser { .. }
| Statement::CreateRole { .. }
| Statement::CreateDatabase { .. }
| Statement::CreateFunction { .. }
| Statement::CreateAggregate { .. }
Expand Down Expand Up @@ -3201,6 +3283,7 @@ pub enum ObjectType {
Source,
Sink,
Database,
Role,
User,
Connection,
Secret,
Expand All @@ -3218,6 +3301,7 @@ impl fmt::Display for ObjectType {
ObjectType::Source => "SOURCE",
ObjectType::Sink => "SINK",
ObjectType::Database => "DATABASE",
ObjectType::Role => "ROLE",
ObjectType::User => "USER",
ObjectType::Secret => "SECRET",
ObjectType::Connection => "CONNECTION",
Expand All @@ -3244,6 +3328,8 @@ impl ParseTo for ObjectType {
ObjectType::Schema
} else if parser.parse_keyword(Keyword::DATABASE) {
ObjectType::Database
} else if parser.parse_keyword(Keyword::ROLE) {
ObjectType::Role
} else if parser.parse_keyword(Keyword::USER) {
ObjectType::User
} else if parser.parse_keyword(Keyword::CONNECTION) {
Expand All @@ -3254,7 +3340,7 @@ impl ParseTo for ObjectType {
ObjectType::Subscription
} else {
return parser.expected(
"TABLE, VIEW, INDEX, MATERIALIZED VIEW, SOURCE, SINK, SUBSCRIPTION, SCHEMA, DATABASE, USER, SECRET or CONNECTION after DROP",
"TABLE, VIEW, INDEX, MATERIALIZED VIEW, SOURCE, SINK, SUBSCRIPTION, SCHEMA, DATABASE, ROLE, USER, SECRET or CONNECTION after DROP",
);
};
Ok(object_type)
Expand Down
Loading
Loading