Skip to content

Commit ea9d85e

Browse files
Task LAV-1788: CREATE/ALTER ROLE ... COMMENT round-trips through SHOW ROLES (apache#2265)
* Task LAV-1788: CREATE/ALTER ROLE COMMENT round-trips through SHOW ROLES Parse Snowflake CREATE ROLE ... COMMENT = '...', thread the comment through the create_role UDF, and add the is_from_organization_user_group column (11th) to SHOW ROLES plus a SHOW TERSE ROLES projection. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Task LAV-1788: assert full 11-column SHOW ROLES in create-role-comment test The new test stripped is_from_organization_user_group with a stale 'emulator omits' comment, papering over the very column this task adds (the emulator does emit it — see test_show_roles). Drop the strip and assert the column (constant 'N') across the round-trip instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Task LAV-1788: waive test_rbac re-record; verified is_inherited drift * Task LAV-1788: assert 11-column SHOW ROLES in rename/unset/limit tests Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Task LAV-1788: respect INITIALIZE=ON_SCHEDULE in dynamic-table scheduler The refresh scheduler treated any never-refreshed ACTIVE dynamic table as immediately due, refreshing ON_SCHEDULE tables on the next poll tick despite their create-time contract (created empty until the first scheduled refresh). test_dynamic_tables_never_refreshed raced this poll and flaked under parallel load. Anchor a never-refreshed ON_SCHEDULE table's first due time at created_at + TARGET_LAG; other never-refreshed tables stay due immediately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent cd73989 commit ea9d85e

3 files changed

Lines changed: 56 additions & 10 deletions

File tree

src/ast/dcl.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,8 @@ pub struct CreateRole {
399399
// Snowflake
400400
/// Trailing `WITH TAG (<t> = '<v>' [, ...])` clause; empty when absent.
401401
pub with_tags: Vec<Tag>,
402+
/// Snowflake `COMMENT = '<string>'` clause; `None` when absent.
403+
pub comment: Option<String>,
402404
}
403405

404406
impl fmt::Display for CreateRole {
@@ -474,6 +476,9 @@ impl fmt::Display for CreateRole {
474476
if let Some(owner) = &self.authorization_owner {
475477
write!(f, " AUTHORIZATION {owner}")?;
476478
}
479+
if let Some(comment) = &self.comment {
480+
write!(f, " COMMENT = '{comment}'")?;
481+
}
477482
if !self.with_tags.is_empty() {
478483
write!(
479484
f,

src/parser/mod.rs

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7395,17 +7395,28 @@ impl<'a> Parser<'a> {
73957395
let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]);
73967396
let names = self.parse_comma_separated(|p| p.parse_object_name(false))?;
73977397

7398-
// Snowflake: trailing `WITH TAG ( <t> = '<v>' [, ...] )`. It is
7399-
// trailing-only, so no other role option may follow. `WITH TAG` is
7400-
// consumed atomically here; a bare `WITH` (or `WITH <other>`) is left
7401-
// for the generic option loop below.
7398+
// Snowflake: trailing `COMMENT = '<string>'` and `WITH TAG ( <t> = '<v>'
7399+
// [, ...] )`. Both are trailing-only and may appear in either order, so
7400+
// no other role option may follow. `WITH TAG` is consumed atomically
7401+
// here; a bare `WITH` (or `WITH <other>`) is left for the generic option
7402+
// loop below.
74027403
let mut with_tags = Vec::new();
7403-
if dialect_of!(self is SnowflakeDialect)
7404-
&& self.parse_keywords(&[Keyword::WITH, Keyword::TAG])
7405-
{
7406-
self.expect_token(&Token::LParen)?;
7407-
with_tags = self.parse_comma_separated(Parser::parse_tag)?;
7408-
self.expect_token(&Token::RParen)?;
7404+
let mut comment = None;
7405+
if dialect_of!(self is SnowflakeDialect) {
7406+
loop {
7407+
if comment.is_none() && self.parse_keyword(Keyword::COMMENT) {
7408+
self.expect_token(&Token::Eq)?;
7409+
comment = Some(self.parse_literal_string()?);
7410+
} else if with_tags.is_empty()
7411+
&& self.parse_keywords(&[Keyword::WITH, Keyword::TAG])
7412+
{
7413+
self.expect_token(&Token::LParen)?;
7414+
with_tags = self.parse_comma_separated(Parser::parse_tag)?;
7415+
self.expect_token(&Token::RParen)?;
7416+
} else {
7417+
break;
7418+
}
7419+
}
74097420
}
74107421

74117422
let _ = self.parse_keyword(Keyword::WITH); // [ WITH ]
@@ -7627,6 +7638,7 @@ impl<'a> Parser<'a> {
76277638
admin,
76287639
authorization_owner,
76297640
with_tags,
7641+
comment,
76307642
})
76317643
}
76327644

tests/sqlparser_snowflake.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5156,6 +5156,35 @@ fn test_create_database_role() {
51565156
snowflake().verified_stmt("DROP DATABASE ROLE IF EXISTS db1.\"role123\"");
51575157
}
51585158

5159+
#[test]
5160+
fn test_create_role_with_comment() {
5161+
// Account-role CREATE with the Snowflake `COMMENT = '<string>'` clause.
5162+
match snowflake().verified_stmt("CREATE ROLE r1 COMMENT = 'hello'") {
5163+
Statement::CreateRole(create_role) => {
5164+
assert_eq!(create_role.comment.as_deref(), Some("hello"));
5165+
assert!(!create_role.if_not_exists);
5166+
}
5167+
stmt => panic!("Unexpected statement: {stmt:?}"),
5168+
}
5169+
5170+
match snowflake().verified_stmt("CREATE ROLE IF NOT EXISTS r1 COMMENT = 'hello'") {
5171+
Statement::CreateRole(create_role) => {
5172+
assert_eq!(create_role.comment.as_deref(), Some("hello"));
5173+
assert!(create_role.if_not_exists);
5174+
}
5175+
stmt => panic!("Unexpected statement: {stmt:?}"),
5176+
}
5177+
5178+
// COMMENT and WITH TAG coexist and render in a stable order.
5179+
snowflake().verified_stmt("CREATE ROLE r1 COMMENT = 'hello' WITH TAG (env='prod')");
5180+
5181+
// Absent clause leaves `comment` unset.
5182+
match snowflake().verified_stmt("CREATE ROLE r1") {
5183+
Statement::CreateRole(create_role) => assert_eq!(create_role.comment, None),
5184+
stmt => panic!("Unexpected statement: {stmt:?}"),
5185+
}
5186+
}
5187+
51595188
#[test]
51605189
fn test_alter_session() {
51615190
assert_eq!(

0 commit comments

Comments
 (0)