Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add PostgreSQL CREATE USER and ALTER USER support #2015

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.

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
ramnes wants to merge 2 commits into apache:main
base: main
Choose a base branch
Loading
from formalco:ramnes/create-alter-user
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
20 changes: 20 additions & 0 deletions src/ast/dcl.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,26 @@ use sqlparser_derive::{Visit, VisitMut};
use super::{display_comma_separated, Expr, Ident, Password};
use crate::ast::{display_separated, ObjectName};

/// Represents whether ROLE or USER keyword was used in CREATE/ALTER statements.
/// In PostgreSQL, CREATE USER and ALTER USER are equivalent to CREATE ROLE and ALTER ROLE,
/// with CREATE USER having LOGIN enabled by default.
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
pub enum RoleKeyword {
Role,
User,
}

impl fmt::Display for RoleKeyword {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
RoleKeyword::Role => write!(f, "ROLE"),
RoleKeyword::User => write!(f, "USER"),
}
}
}

/// An option in `ROLE` statement.
///
/// <https://www.postgresql.org/docs/current/sql-createrole.html>
Expand Down
18 changes: 14 additions & 4 deletions src/ast/mod.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub use self::data_type::{
ExactNumberInfo, IntervalFields, StructBracketKind, TimezoneInfo,
};
pub use self::dcl::{
AlterRoleOperation, ResetConfig, RoleOption, SecondaryRoles, SetConfigValue, Use,
AlterRoleOperation, ResetConfig, RoleKeyword, RoleOption, SecondaryRoles, SetConfigValue, Use,
};
pub use self::ddl::{
AlterColumnOperation, AlterConnectorOwner, AlterIndexOperation, AlterPolicyOperation,
Expand Down Expand Up @@ -3314,6 +3314,8 @@ pub enum Statement {
CreateRole {
names: Vec<ObjectName>,
if_not_exists: bool,
/// Whether ROLE or USER keyword was used
keyword: RoleKeyword,
Comment on lines +3317 to +3318
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use something like this representation instead? I think its a somewhat more invasive changes but clearer API wise.

struct CreateUserOrRole {
 names: Vec<ObjectName>,
 if_not_exists: bool,
 // Postgres
 login: Option<bool>,
 inherit: Option<bool>,
 // ...
}
Statement::CreateUser(CreateUserOrRole)
Statement::CreateRole(CreateUserOrRole)
struct AlterUserOrRole {
 name: Ident
}
Statement::AlterUser(AlterUserOrRole)
Statement::AlterRole(AlterUserOrRole)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a namespace problem here, because there's already a CreateUser structure for Snowflake semantics. In the grand scheme of things, CreateUser and CreateRole should probably be both compatible with both Snowflake and PostgreSQL? I'm not sure if I'm seasoned enough with the codebase to bring that in, and that would be a very different kind of work than this PR. :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I'm not sure I followed, do we mean snowflake has CreateUser supported by the parser already?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but as far as I understand, it's a very different syntax / semantics. Postgres "USER" is really just an alias for "ROLE", and Postgres semantics are currently handled by CreateRole and AlterRole. Which is why I just added the keyword here: it doesn't introduce any backwards compatibility. It's just more data to the AST for an already existing expression parsing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this would probably need a refactor so that all dialects use the same structures for ALTER / CREATE USER / ROLE, but I guess it would make more sense in a second step – the inconsistency is already there. This PR fixes a currently broken expression parsing "for free".

// Postgres
login: Option<bool>,
inherit: Option<bool>,
Expand Down Expand Up @@ -3421,6 +3423,8 @@ pub enum Statement {
/// ```
AlterRole {
name: Ident,
/// Whether ROLE or USER keyword was used
keyword: RoleKeyword,
operation: AlterRoleOperation,
},
/// ```sql
Expand Down Expand Up @@ -5279,6 +5283,7 @@ impl fmt::Display for Statement {
Statement::CreateRole {
names,
if_not_exists,
keyword,
inherit,
login,
bypassrls,
Expand All @@ -5298,7 +5303,7 @@ impl fmt::Display for Statement {
} => {
write!(
f,
"CREATE ROLE {if_not_exists}{names}{superuser}{create_db}{create_role}{inherit}{login}{replication}{bypassrls}",
"CREATE {keyword} {if_not_exists}{names}{superuser}{create_db}{create_role}{inherit}{login}{replication}{bypassrls}",
if_not_exists = if *if_not_exists { "IF NOT EXISTS " } else { "" },
names = display_separated(names, ", "),
superuser = match *superuser {
Expand Down Expand Up @@ -5337,6 +5342,7 @@ impl fmt::Display for Statement {
None => ""
}
)?;

if let Some(limit) = connection_limit {
write!(f, " CONNECTION LIMIT {limit}")?;
}
Expand Down Expand Up @@ -5506,8 +5512,12 @@ impl fmt::Display for Statement {
Statement::AlterType(AlterType { name, operation }) => {
write!(f, "ALTER TYPE {name} {operation}")
}
Statement::AlterRole { name, operation } => {
write!(f, "ALTER ROLE {name} {operation}")
Statement::AlterRole {
name,
keyword,
operation,
} => {
write!(f, "ALTER {keyword} {name} {operation}")
}
Statement::AlterPolicy {
name,
Expand Down
19 changes: 18 additions & 1 deletion src/parser/alter.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use super::{Parser, ParserError};
use crate::{
ast::{
AlterConnectorOwner, AlterPolicyOperation, AlterRoleOperation, Expr, Password, ResetConfig,
RoleOption, SetConfigValue, Statement,
RoleKeyword, RoleOption, SetConfigValue, Statement,
},
dialect::{MsSqlDialect, PostgreSqlDialect},
keywords::Keyword,
Expand All @@ -39,6 +39,10 @@ impl Parser<'_> {
))
}

pub fn parse_alter_user(&mut self) -> Result<Statement, ParserError> {
self.parse_pg_alter_user()
}

/// Parse ALTER POLICY statement
/// ```sql
/// ALTER POLICY policy_name ON table_name [ RENAME TO new_name ]
Expand Down Expand Up @@ -162,11 +166,23 @@ impl Parser<'_> {

Ok(Statement::AlterRole {
name: role_name,
keyword: RoleKeyword::Role,
operation,
})
}

fn parse_pg_alter_role(&mut self) -> Result<Statement, ParserError> {
self.parse_pg_alter_role_or_user(RoleKeyword::Role)
}

fn parse_pg_alter_user(&mut self) -> Result<Statement, ParserError> {
self.parse_pg_alter_role_or_user(RoleKeyword::User)
}

fn parse_pg_alter_role_or_user(
&mut self,
keyword: RoleKeyword,
) -> Result<Statement, ParserError> {
let role_name = self.parse_identifier()?;

// [ IN DATABASE _`database_name`_ ]
Expand Down Expand Up @@ -246,6 +262,7 @@ impl Parser<'_> {

Ok(Statement::AlterRole {
name: role_name,
keyword,
operation,
})
}
Expand Down
23 changes: 23 additions & 0 deletions src/parser/mod.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4788,6 +4788,10 @@ impl<'a> Parser<'a> {
}

fn parse_create_user(&mut self, or_replace: bool) -> Result<Statement, ParserError> {
if dialect_of!(self is PostgreSqlDialect) {
return self.parse_pg_create_user();
}

let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]);
let name = self.parse_identifier()?;
let options = self.parse_key_value_options(false, &[Keyword::WITH, Keyword::TAG])?;
Expand Down Expand Up @@ -5995,6 +5999,17 @@ impl<'a> Parser<'a> {
}

pub fn parse_create_role(&mut self) -> Result<Statement, ParserError> {
self.parse_pg_create_role_or_user(RoleKeyword::Role)
}

pub fn parse_pg_create_user(&mut self) -> Result<Statement, ParserError> {
self.parse_pg_create_role_or_user(RoleKeyword::User)
}

fn parse_pg_create_role_or_user(
&mut self,
keyword: RoleKeyword,
) -> Result<Statement, ParserError> {
let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]);
let names = self.parse_comma_separated(|p| p.parse_object_name(false))?;

Expand Down Expand Up @@ -6196,9 +6211,15 @@ impl<'a> Parser<'a> {
}?
}

// In PostgreSQL, CREATE USER defaults to LOGIN=true, CREATE ROLE defaults to LOGIN=false
if login.is_none() && keyword == RoleKeyword::User {
login = Some(true);
}

Ok(Statement::CreateRole {
names,
if_not_exists,
keyword,
login,
inherit,
bypassrls,
Expand Down Expand Up @@ -9263,6 +9284,7 @@ impl<'a> Parser<'a> {
Keyword::TABLE,
Keyword::INDEX,
Keyword::ROLE,
Keyword::USER,
Keyword::POLICY,
Keyword::CONNECTOR,
Keyword::ICEBERG,
Expand Down Expand Up @@ -9300,6 +9322,7 @@ impl<'a> Parser<'a> {
})
}
Keyword::ROLE => self.parse_alter_role(),
Keyword::USER => self.parse_alter_user(),
Keyword::POLICY => self.parse_alter_policy(),
Keyword::CONNECTOR => self.parse_alter_connector(),
// unreachable because expect_one_of_keywords used above
Expand Down
20 changes: 12 additions & 8 deletions tests/sqlparser_common.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16600,21 +16600,25 @@ fn parse_odbc_time_date_timestamp() {

#[test]
fn parse_create_user() {
let create = verified_stmt("CREATE USER u1");
// PostgreSQL has different CREATE USER semantics (CREATE ROLE with LOGIN=true),
// so we exclude it from this test which is for Snowflake-style CREATE USER
let dialects = all_dialects_except(|d| d.is::<PostgreSqlDialect>());

let create = dialects.verified_stmt("CREATE USER u1");
match create {
Statement::CreateUser(stmt) => {
assert_eq!(stmt.name, Ident::new("u1"));
}
_ => unreachable!(),
}
verified_stmt("CREATE OR REPLACE USER u1");
verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1");
verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret'");
verified_stmt(
dialects.verified_stmt("CREATE OR REPLACE USER u1");
dialects.verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1");
dialects.verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret'");
dialects.verified_stmt(
"CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE",
);
verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE TAG (t1='v1')");
let create = verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE WITH TAG (t1='v1', t2='v2')");
dialects.verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE TAG (t1='v1')");
let create = dialects.verified_stmt("CREATE OR REPLACE USER IF NOT EXISTS u1 PASSWORD='secret' MUST_CHANGE_PASSWORD=TRUE TYPE=SERVICE WITH TAG (t1='v1', t2='v2')");
match create {
Statement::CreateUser(stmt) => {
assert_eq!(stmt.name, Ident::new("u1"));
Expand Down Expand Up @@ -16982,7 +16986,7 @@ fn test_parse_semantic_view_table_factor() {
}

let ast_sql = r#"SELECT * FROM SEMANTIC_VIEW(
my_model
my_model
DIMENSIONS DATE_PART('year', date_col), region_name
METRICS orders.revenue, orders.count
WHERE active = true
Expand Down
3 changes: 3 additions & 0 deletions tests/sqlparser_mssql.rs
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,7 @@ fn parse_alter_role() {
quote_style: None,
span: Span::empty(),
},
keyword: RoleKeyword::Role,
operation: AlterRoleOperation::RenameRole {
role_name: Ident {
value: "new_name".into(),
Expand All @@ -821,6 +822,7 @@ fn parse_alter_role() {
quote_style: None,
span: Span::empty(),
},
keyword: RoleKeyword::Role,
operation: AlterRoleOperation::AddMember {
member_name: Ident {
value: "new_member".into(),
Expand All @@ -840,6 +842,7 @@ fn parse_alter_role() {
quote_style: None,
span: Span::empty(),
},
keyword: RoleKeyword::Role,
operation: AlterRoleOperation::DropMember {
member_name: Ident {
value: "old_member".into(),
Expand Down
Loading

AltStyle によって変換されたページ (->オリジナル) /