Skip to content

Re-enable trailing commas in DCL #1318

New issue

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

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

Already on GitHub? # to your account

Merged
merged 5 commits into from
Jul 8, 2024
Merged
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
39 changes: 29 additions & 10 deletions src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ pub enum ParserError {
RecursionLimitExceeded,
}

// avoid clippy type_complexity warnings
type ParsedAction = (Keyword, Option<Vec<Ident>>);

// Use `Parser::expected` instead, if possible
macro_rules! parser_err {
($MSG:expr, $loc:expr) => {
Expand Down Expand Up @@ -3278,6 +3281,29 @@ impl<'a> Parser<'a> {
ret
}

pub fn parse_actions_list(&mut self) -> Result<Vec<ParsedAction>, ParserError> {
let mut values = vec![];
loop {
values.push(self.parse_grant_permission()?);
if !self.consume_token(&Token::Comma) {
break;
} else if self.options.trailing_commas {
match self.peek_token().token {
Token::Word(kw) if kw.keyword == Keyword::ON => {
break;
}
Token::RParen
| Token::SemiColon
| Token::EOF
| Token::RBracket
| Token::RBrace => break,
_ => continue,
}
}
}
Ok(values)
}

/// Parse a comma-separated list of 1+ items accepted by `F`
pub fn parse_comma_separated<T, F>(&mut self, mut f: F) -> Result<Vec<T>, ParserError>
where
Expand All @@ -3291,9 +3317,7 @@ impl<'a> Parser<'a> {
} else if self.options.trailing_commas {
match self.peek_token().token {
Token::Word(kw)
if keywords::RESERVED_FOR_COLUMN_ALIAS
.iter()
.any(|d| kw.keyword == *d) =>
if keywords::RESERVED_FOR_COLUMN_ALIAS.contains(&kw.keyword) =>
{
break;
}
Expand Down Expand Up @@ -9556,11 +9580,8 @@ impl<'a> Parser<'a> {
with_privileges_keyword: self.parse_keyword(Keyword::PRIVILEGES),
}
} else {
let old_value = self.options.trailing_commas;
self.options.trailing_commas = false;

let (actions, err): (Vec<_>, Vec<_>) = self
.parse_comma_separated(Parser::parse_grant_permission)?
.parse_actions_list()?
.into_iter()
.map(|(kw, columns)| match kw {
Keyword::DELETE => Ok(Action::Delete),
Expand All @@ -9582,8 +9603,6 @@ impl<'a> Parser<'a> {
})
.partition(Result::is_ok);

self.options.trailing_commas = old_value;

if !err.is_empty() {
let errors: Vec<Keyword> = err.into_iter().filter_map(|x| x.err()).collect();
return Err(ParserError::ParserError(format!(
Expand Down Expand Up @@ -9629,7 +9648,7 @@ impl<'a> Parser<'a> {
Ok((privileges, objects))
}

pub fn parse_grant_permission(&mut self) -> Result<(Keyword, Option<Vec<Ident>>), ParserError> {
pub fn parse_grant_permission(&mut self) -> Result<ParsedAction, ParserError> {
if let Some(kw) = self.parse_one_of_keywords(&[
Keyword::CONNECT,
Keyword::CREATE,
Expand Down
12 changes: 12 additions & 0 deletions tests/sqlparser_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8894,6 +8894,11 @@ fn parse_trailing_comma() {
"CREATE TABLE employees (name TEXT, age INT)",
);

trailing_commas.one_statement_parses_to(
"GRANT USAGE, SELECT, INSERT, ON p TO u",
"GRANT USAGE, SELECT, INSERT ON p TO u",
);

trailing_commas.verified_stmt("SELECT album_id, name FROM track");

trailing_commas.verified_stmt("SELECT * FROM track ORDER BY milliseconds");
Expand All @@ -8913,6 +8918,13 @@ fn parse_trailing_comma() {
ParserError::ParserError("Expected an expression, found: from".to_string())
);

assert_eq!(
trailing_commas
.parse_sql_statements("REVOKE USAGE, SELECT, ON p TO u")
.unwrap_err(),
ParserError::ParserError("Expected a privilege keyword, found: ON".to_string())
);

assert_eq!(
trailing_commas
.parse_sql_statements("CREATE TABLE employees (name text, age int,)")
Expand Down
Loading