From 95d2b361c4e9ff0dc41c2bca8537ced355719fd0 Mon Sep 17 00:00:00 2001 From: Mohamed Abdeen Date: Tue, 18 Jun 2024 01:20:58 +0300 Subject: [PATCH 1/5] initial change --- src/keywords.rs | 31 +++++++++++++++++++++++++++++++ src/parser/mod.rs | 38 +++++++++++++++++++++++++++++--------- tests/sqlparser_common.rs | 12 ++++++++++++ 3 files changed, 72 insertions(+), 9 deletions(-) diff --git a/src/keywords.rs b/src/keywords.rs index e75d45e44..d3c6a4735 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -886,3 +886,34 @@ pub const RESERVED_FOR_COLUMN_ALIAS: &[Keyword] = &[ Keyword::INTO, Keyword::END, ]; + +/// Can't be used as a column alias, so that `SELECT alias` +/// can be parsed unambiguously without looking ahead. +pub const RESERVED_FOR_GRANT_PERMISSIONS: &[Keyword] = &[ + // Reserved as both a table and a column alias: + Keyword::ON, + Keyword::WITH, + Keyword::EXPLAIN, + Keyword::ANALYZE, + Keyword::WHERE, + Keyword::GROUP, + Keyword::SORT, + Keyword::HAVING, + Keyword::ORDER, + Keyword::TOP, + Keyword::LATERAL, + Keyword::VIEW, + Keyword::LIMIT, + Keyword::OFFSET, + Keyword::FETCH, + Keyword::UNION, + Keyword::EXCEPT, + Keyword::INTERSECT, + Keyword::CLUSTER, + Keyword::DISTRIBUTE, + Keyword::RETURNING, + // Reserved only as a column alias in the `SELECT` clause + Keyword::FROM, + Keyword::INTO, + Keyword::END, +]; diff --git a/src/parser/mod.rs b/src/parser/mod.rs index e240441b9..0a076077c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -3278,6 +3278,33 @@ impl<'a> Parser<'a> { ret } + pub fn parse_actions_list( + &mut self, + ) -> Result>)>, 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 keywords::RESERVED_FOR_GRANT_PERMISSIONS.contains(&kw.keyword) => + { + 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(&mut self, mut f: F) -> Result, ParserError> where @@ -3291,9 +3318,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; } @@ -9556,11 +9581,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), @@ -9582,8 +9604,6 @@ impl<'a> Parser<'a> { }) .partition(Result::is_ok); - self.options.trailing_commas = old_value; - if !err.is_empty() { let errors: Vec = err.into_iter().filter_map(|x| x.err()).collect(); return Err(ParserError::ParserError(format!( diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index a86858129..6439abdd2 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -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"); @@ -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,)") From d69da274b8142e2c034e9c5e5c0927516718925d Mon Sep 17 00:00:00 2001 From: Mohamed Abdeen Date: Tue, 18 Jun 2024 01:34:21 +0300 Subject: [PATCH 2/5] revert new kw slice --- src/keywords.rs | 31 ------------------------------- src/parser/mod.rs | 4 +--- 2 files changed, 1 insertion(+), 34 deletions(-) diff --git a/src/keywords.rs b/src/keywords.rs index d3c6a4735..e75d45e44 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -886,34 +886,3 @@ pub const RESERVED_FOR_COLUMN_ALIAS: &[Keyword] = &[ Keyword::INTO, Keyword::END, ]; - -/// Can't be used as a column alias, so that `SELECT alias` -/// can be parsed unambiguously without looking ahead. -pub const RESERVED_FOR_GRANT_PERMISSIONS: &[Keyword] = &[ - // Reserved as both a table and a column alias: - Keyword::ON, - Keyword::WITH, - Keyword::EXPLAIN, - Keyword::ANALYZE, - Keyword::WHERE, - Keyword::GROUP, - Keyword::SORT, - Keyword::HAVING, - Keyword::ORDER, - Keyword::TOP, - Keyword::LATERAL, - Keyword::VIEW, - Keyword::LIMIT, - Keyword::OFFSET, - Keyword::FETCH, - Keyword::UNION, - Keyword::EXCEPT, - Keyword::INTERSECT, - Keyword::CLUSTER, - Keyword::DISTRIBUTE, - Keyword::RETURNING, - // Reserved only as a column alias in the `SELECT` clause - Keyword::FROM, - Keyword::INTO, - Keyword::END, -]; diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 0a076077c..1756001e8 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -3288,9 +3288,7 @@ impl<'a> Parser<'a> { break; } else if self.options.trailing_commas { match self.peek_token().token { - Token::Word(kw) - if keywords::RESERVED_FOR_GRANT_PERMISSIONS.contains(&kw.keyword) => - { + Token::Word(kw) if kw.keyword == Keyword::ON => { break; } Token::RParen From a76052d61d49718fa23a6b2a70b5babb2bd3cd7b Mon Sep 17 00:00:00 2001 From: Mohamed Abdeen Date: Tue, 18 Jun 2024 01:41:07 +0300 Subject: [PATCH 3/5] fix test --- tests/sqlparser_common.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 6439abdd2..3bd920077 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -8895,7 +8895,7 @@ fn parse_trailing_comma() { ); trailing_commas.one_statement_parses_to( - "GRANT USAGE, SELECT, INSERT ON p TO u", + "GRANT USAGE, SELECT, INSERT, ON p TO u", "GRANT USAGE, SELECT, INSERT ON p TO u", ); From 12e7fc9155c2de7bd0fba26b04a9faf38b6e03a0 Mon Sep 17 00:00:00 2001 From: Mohamed Abdeen Date: Tue, 18 Jun 2024 01:49:43 +0300 Subject: [PATCH 4/5] ignore clippy type complexity warning --- src/parser/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 1756001e8..b4527fd5c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -3278,6 +3278,7 @@ impl<'a> Parser<'a> { ret } + #[allow(clippy::type_complexity)] pub fn parse_actions_list( &mut self, ) -> Result>)>, ParserError> { From a8e29232948f6559b5ee99cd508060f5353632a4 Mon Sep 17 00:00:00 2001 From: Mohamed Abdeen Date: Tue, 18 Jun 2024 01:57:44 +0300 Subject: [PATCH 5/5] alias type for clippy --- src/parser/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index b4527fd5c..eb8e72014 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -42,6 +42,9 @@ pub enum ParserError { RecursionLimitExceeded, } +// avoid clippy type_complexity warnings +type ParsedAction = (Keyword, Option>); + // Use `Parser::expected` instead, if possible macro_rules! parser_err { ($MSG:expr, $loc:expr) => { @@ -3278,10 +3281,7 @@ impl<'a> Parser<'a> { ret } - #[allow(clippy::type_complexity)] - pub fn parse_actions_list( - &mut self, - ) -> Result>)>, ParserError> { + pub fn parse_actions_list(&mut self) -> Result, ParserError> { let mut values = vec![]; loop { values.push(self.parse_grant_permission()?); @@ -9648,7 +9648,7 @@ impl<'a> Parser<'a> { Ok((privileges, objects)) } - pub fn parse_grant_permission(&mut self) -> Result<(Keyword, Option>), ParserError> { + pub fn parse_grant_permission(&mut self) -> Result { if let Some(kw) = self.parse_one_of_keywords(&[ Keyword::CONNECT, Keyword::CREATE,