Skip to content

MsSql: Serialize DECLARE variable lists and WITH NAME role renames correctly - #2702

Closed
princejain756 wants to merge 2 commits into
apache:mainfrom
princejain756:fix-declare-alter-role-serialize
Closed

princejain756 wants to merge 2 commits into
apache:mainfrom
princejain756:fix-declare-alter-role-serialize

Conversation

@princejain756

Copy link
Copy Markdown

Two MsSqlDialect AST nodes serialized into SQL their own dialect cannot parse back:

// DECLARE with a comma-separated variable list:
Parser::parse_sql(&MsSqlDialect {}, "DECLARE @a INT, @b INT")
    .unwrap()[0].to_string()
// -> "DECLARE @a INT; @b INT"  (the tail fails to reparse)

// ALTER ROLE rename:
Parser::parse_sql(&MsSqlDialect {}, "ALTER ROLE x WITH NAME = y")
    .unwrap()[0].to_string()
// -> "ALTER ROLE x RENAME TO y"  (MsSql only accepts WITH NAME =)

Statement::Declare now carries a DeclareDelimiter: Comma for the MsSql, BigQuery and PostgreSQL declaration forms, Semicolon for Snowflake's DECLARE a; b blocks. The T-SQL rename maps to a new AlterRoleOperation::RenameRoleWithName variant that emits WITH NAME =, matching how AddMember/DropMember already keep their T-SQL spellings.

Verified against the T-SQL docs for DECLARE and ALTER ROLE.

AI assistance disclosure: this change was developed with AI coding assistance.

…rrectly

Two MsSql AST nodes serialized in a form their own dialect cannot
parse:

- DECLARE @A INT, @b INT collected each variable into
  Statement::Declare::stmts, which Display joined with '; ' so the
  output reparsed as separate statements and the tail failed to parse.
  Statement::Declare now carries a DeclareDelimiter: Comma for MsSql,
  BigQuery and PostgreSQL declarations, Semicolon for Snowflake's
  DECLARE a; b blocks.

- ALTER ROLE x WITH NAME = y mapped to AlterRoleOperation::RenameRole,
  which serializes the PostgreSQL RENAME TO syntax that MsSqlDialect
  rejects. The WITH NAME = spelling now parses to a dedicated
  RenameRoleWithName variant that round-trips under MsSqlDialect.
@princejain756

Copy link
Copy Markdown
Author

Closing this: the DECLARE half landed upstream via #2671 (comma preservation in displayed DECLARE lists), and the ALTER ROLE half is now #2709, which carries the same fix as a focused atomic PR using a use_with_name flag on RenameRole rather than a dedicated variant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant