From cad074203f519996ed8208b5840c1e4d2036e1e1 Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Fri, 4 Oct 2024 14:04:38 +0200 Subject: [PATCH 1/6] Add support for subqueries in quantified comparison predicates(ANY/ALL/SOME) --- src/ast/mod.rs | 24 ++++++++++++++++++++++-- src/parser/mod.rs | 21 +++++++++++++++++---- tests/sqlparser_common.rs | 8 ++++++++ 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 8c4bc25af..b0b0902b0 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -650,6 +650,8 @@ pub enum Expr { left: Box, compare_op: BinaryOperator, right: Box, + // ANY and SOME are synonymous + is_some: bool, }, /// `ALL` operation e.g. `foo > ALL(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` AllOp { @@ -1332,12 +1334,30 @@ impl fmt::Display for Expr { left, compare_op, right, - } => write!(f, "{left} {compare_op} ANY({right})"), + is_some, + } => { + let add_parens = !matches!(right.as_ref(), Expr::Subquery(_)); + write!( + f, + "{left} {compare_op} {}{}{right}{}", + if *is_some { "SOME" } else { "ANY" }, + if add_parens { "(" } else { "" }, + if add_parens { ")" } else { "" }, + ) + } Expr::AllOp { left, compare_op, right, - } => write!(f, "{left} {compare_op} ALL({right})"), + } => { + let add_parens = !matches!(right.as_ref(), Expr::Subquery(_)); + write!( + f, + "{left} {compare_op} ALL{}{right}{}", + if add_parens { "(" } else { "" }, + if add_parens { ")" } else { "" }, + ) + } Expr::UnaryOp { op, expr } => { if op == &UnaryOperator::PGPostfixFactorial { write!(f, "{expr}{op}") diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 88c3bd19c..6bdbbb62d 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -2639,10 +2639,22 @@ impl<'a> Parser<'a> { }; if let Some(op) = regular_binary_operator { - if let Some(keyword) = self.parse_one_of_keywords(&[Keyword::ANY, Keyword::ALL]) { + if let Some(keyword) = + self.parse_one_of_keywords(&[Keyword::ANY, Keyword::ALL, Keyword::SOME]) + { self.expect_token(&Token::LParen)?; - let right = self.parse_subexpr(precedence)?; - self.expect_token(&Token::RParen)?; + let right = if self.parse_keyword(Keyword::SELECT) { + // We have a subquery ahead (SELECT ...) need to rewind and + // use the parenthesis for parsing the subquery. + self.prev_token(); // SELECT + self.prev_token(); // LParen + self.parse_subexpr(precedence)? + } else { + // Non-subquery exp + let right = self.parse_subexpr(precedence)?; + self.expect_token(&Token::RParen)?; + right + }; if !matches!( op, @@ -2667,10 +2679,11 @@ impl<'a> Parser<'a> { compare_op: op, right: Box::new(right), }, - Keyword::ANY => Expr::AnyOp { + Keyword::ANY | Keyword::SOME => Expr::AnyOp { left: Box::new(expr), compare_op: op, right: Box::new(right), + is_some: keyword == Keyword::SOME, }, _ => unreachable!(), }) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 5d5a17ca5..c59a2b09f 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1907,6 +1907,7 @@ fn parse_binary_any() { left: Box::new(Expr::Identifier(Ident::new("a"))), compare_op: BinaryOperator::Eq, right: Box::new(Expr::Identifier(Ident::new("b"))), + is_some: false, }), select.projection[0] ); @@ -11309,3 +11310,10 @@ fn test_select_where_with_like_or_ilike_any() { verified_stmt(r#"SELECT * FROM x WHERE a ILIKE ANY ('%Jo%oe%', 'T%e')"#); verified_stmt(r#"SELECT * FROM x WHERE a LIKE ANY ('%Jo%oe%', 'T%e')"#); } + +#[test] +fn parse_any_some_all_comparison() { + verified_stmt("SELECT c1 FROM tbl WHERE c1 = ANY(SELECT c2 FROM tbl)"); + verified_stmt("SELECT c1 FROM tbl WHERE c1 >= ALL(SELECT c2 FROM tbl)"); + verified_stmt("SELECT c1 FROM tbl WHERE c1 <> SOME(SELECT c2 FROM tbl)"); +} From 76dbe35f5bcbd7127b7f53daa3617c2853585ff1 Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Sat, 5 Oct 2024 21:32:26 +0200 Subject: [PATCH 2/6] Code reuse and more testing --- src/parser/mod.rs | 30 +++++++++++++----------------- tests/sqlparser_common.rs | 3 ++- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 6bdbbb62d..5f4e45f00 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -1302,10 +1302,7 @@ impl<'a> Parser<'a> { } fn try_parse_expr_sub_query(&mut self) -> Result, ParserError> { - if self - .parse_one_of_keywords(&[Keyword::SELECT, Keyword::WITH]) - .is_none() - { + if !self.is_query_ahead() { return Ok(None); } self.prev_token(); @@ -1334,11 +1331,7 @@ impl<'a> Parser<'a> { // Snowflake permits a subquery to be passed as an argument without // an enclosing set of parens if it's the only argument. - if dialect_of!(self is SnowflakeDialect) - && self - .parse_one_of_keywords(&[Keyword::WITH, Keyword::SELECT]) - .is_some() - { + if dialect_of!(self is SnowflakeDialect) && self.is_query_ahead() { self.prev_token(); let subquery = self.parse_boxed_query()?; self.expect_token(&Token::RParen)?; @@ -2643,14 +2636,14 @@ impl<'a> Parser<'a> { self.parse_one_of_keywords(&[Keyword::ANY, Keyword::ALL, Keyword::SOME]) { self.expect_token(&Token::LParen)?; - let right = if self.parse_keyword(Keyword::SELECT) { - // We have a subquery ahead (SELECT ...) need to rewind and - // use the parenthesis for parsing the subquery. + let right = if self.is_query_ahead() { + // We have a subquery ahead (SELECT\WITH ...) need to rewind and + // use the parenthesis for parsing the subquery as an expression. self.prev_token(); // SELECT self.prev_token(); // LParen self.parse_subexpr(precedence)? } else { - // Non-subquery exp + // Non-subquery expression let right = self.parse_subexpr(precedence)?; self.expect_token(&Token::RParen)?; right @@ -10484,10 +10477,7 @@ impl<'a> Parser<'a> { vec![] }; PivotValueSource::Any(order_by) - } else if self - .parse_one_of_keywords(&[Keyword::SELECT, Keyword::WITH]) - .is_some() - { + } else if self.is_query_ahead() { self.prev_token(); PivotValueSource::Subquery(self.parse_query()?) } else { @@ -12154,6 +12144,12 @@ impl<'a> Parser<'a> { pub fn into_tokens(self) -> Vec { self.tokens } + + /// Checks if the next keyword indicates a query, i.e. SELECT or WITH + pub fn is_query_ahead(&mut self) -> bool { + self.parse_one_of_keywords(&[Keyword::SELECT, Keyword::WITH]) + .is_some() + } } impl Word { diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index c59a2b09f..9c73fd9db 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -11312,8 +11312,9 @@ fn test_select_where_with_like_or_ilike_any() { } #[test] -fn parse_any_some_all_comparison() { +fn test_any_some_all_comparison() { verified_stmt("SELECT c1 FROM tbl WHERE c1 = ANY(SELECT c2 FROM tbl)"); verified_stmt("SELECT c1 FROM tbl WHERE c1 >= ALL(SELECT c2 FROM tbl)"); verified_stmt("SELECT c1 FROM tbl WHERE c1 <> SOME(SELECT c2 FROM tbl)"); + verified_stmt("SELECT 1 = ANY(WITH x AS (SELECT 1) SELECT * FROM x)"); } From 3860cb5ce318b71106177e6ea63fe42e361293d6 Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Tue, 8 Oct 2024 09:01:58 +0200 Subject: [PATCH 3/6] Code review comments --- src/ast/mod.rs | 2 ++ src/parser/mod.rs | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index b0b0902b0..920682ff8 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -646,6 +646,7 @@ pub enum Expr { regexp: bool, }, /// `ANY` operation e.g. `foo > ANY(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` + /// https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#_8_9_quantified_comparison_predicate AnyOp { left: Box, compare_op: BinaryOperator, @@ -654,6 +655,7 @@ pub enum Expr { is_some: bool, }, /// `ALL` operation e.g. `foo > ALL(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` + /// https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#_8_9_quantified_comparison_predicate AllOp { left: Box, compare_op: BinaryOperator, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 5f4e45f00..3775b8124 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -1305,7 +1305,6 @@ impl<'a> Parser<'a> { if !self.is_query_ahead() { return Ok(None); } - self.prev_token(); Ok(Some(Expr::Subquery(self.parse_boxed_query()?))) } @@ -1332,7 +1331,6 @@ impl<'a> Parser<'a> { // Snowflake permits a subquery to be passed as an argument without // an enclosing set of parens if it's the only argument. if dialect_of!(self is SnowflakeDialect) && self.is_query_ahead() { - self.prev_token(); let subquery = self.parse_boxed_query()?; self.expect_token(&Token::RParen)?; return Ok(Expr::Function(Function { @@ -2639,7 +2637,6 @@ impl<'a> Parser<'a> { let right = if self.is_query_ahead() { // We have a subquery ahead (SELECT\WITH ...) need to rewind and // use the parenthesis for parsing the subquery as an expression. - self.prev_token(); // SELECT self.prev_token(); // LParen self.parse_subexpr(precedence)? } else { @@ -10478,7 +10475,6 @@ impl<'a> Parser<'a> { }; PivotValueSource::Any(order_by) } else if self.is_query_ahead() { - self.prev_token(); PivotValueSource::Subquery(self.parse_query()?) } else { PivotValueSource::List(self.parse_comma_separated(Self::parse_expr_with_alias)?) @@ -12145,10 +12141,16 @@ impl<'a> Parser<'a> { self.tokens } - /// Checks if the next keyword indicates a query, i.e. SELECT or WITH - pub fn is_query_ahead(&mut self) -> bool { - self.parse_one_of_keywords(&[Keyword::SELECT, Keyword::WITH]) + /// Returns true if the next keyword indicates a sub query, i.e. SELECT or WITH + fn is_query_ahead(&mut self) -> bool { + if self + .parse_one_of_keywords(&[Keyword::SELECT, Keyword::WITH]) .is_some() + { + self.prev_token(); + return true; + } + false } } From e4def244f2417c2ced8cec1ed59588332d57c6c8 Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Tue, 8 Oct 2024 09:26:15 +0200 Subject: [PATCH 4/6] Code review comments --- 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 3775b8124..f161a2f99 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -1302,7 +1302,7 @@ impl<'a> Parser<'a> { } fn try_parse_expr_sub_query(&mut self) -> Result, ParserError> { - if !self.is_query_ahead() { + if !self.peek_sub_query() { return Ok(None); } @@ -1330,7 +1330,7 @@ impl<'a> Parser<'a> { // Snowflake permits a subquery to be passed as an argument without // an enclosing set of parens if it's the only argument. - if dialect_of!(self is SnowflakeDialect) && self.is_query_ahead() { + if dialect_of!(self is SnowflakeDialect) && self.peek_sub_query() { let subquery = self.parse_boxed_query()?; self.expect_token(&Token::RParen)?; return Ok(Expr::Function(Function { @@ -2634,7 +2634,7 @@ impl<'a> Parser<'a> { self.parse_one_of_keywords(&[Keyword::ANY, Keyword::ALL, Keyword::SOME]) { self.expect_token(&Token::LParen)?; - let right = if self.is_query_ahead() { + let right = if self.peek_sub_query() { // We have a subquery ahead (SELECT\WITH ...) need to rewind and // use the parenthesis for parsing the subquery as an expression. self.prev_token(); // LParen @@ -10474,7 +10474,7 @@ impl<'a> Parser<'a> { vec![] }; PivotValueSource::Any(order_by) - } else if self.is_query_ahead() { + } else if self.peek_sub_query() { PivotValueSource::Subquery(self.parse_query()?) } else { PivotValueSource::List(self.parse_comma_separated(Self::parse_expr_with_alias)?) @@ -12142,7 +12142,7 @@ impl<'a> Parser<'a> { } /// Returns true if the next keyword indicates a sub query, i.e. SELECT or WITH - fn is_query_ahead(&mut self) -> bool { + fn peek_sub_query(&mut self) -> bool { if self .parse_one_of_keywords(&[Keyword::SELECT, Keyword::WITH]) .is_some() From 704fa97b27f7a6d1ca94aefb3868370a7a5bc64a Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Tue, 8 Oct 2024 18:39:50 +0200 Subject: [PATCH 5/6] Improve docs --- src/ast/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 920682ff8..989be2880 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -646,16 +646,16 @@ pub enum Expr { regexp: bool, }, /// `ANY` operation e.g. `foo > ANY(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` - /// https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#_8_9_quantified_comparison_predicate + /// https://docs.snowflake.com/en/sql-reference/operators-subquery#all-any AnyOp { left: Box, compare_op: BinaryOperator, right: Box, - // ANY and SOME are synonymous + // ANY and SOME are synonymous: https://docs.cloudera.com/cdw-runtime/cloud/using-hiveql/topics/hive_comparison_predicates.html is_some: bool, }, /// `ALL` operation e.g. `foo > ALL(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` - /// https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#_8_9_quantified_comparison_predicate + /// https://docs.snowflake.com/en/sql-reference/operators-subquery#all-any AllOp { left: Box, compare_op: BinaryOperator, From 2219f8ecc548f8968f207453d8a840b87b89e0fa Mon Sep 17 00:00:00 2001 From: Yoav Cohen Date: Wed, 9 Oct 2024 21:25:20 +0200 Subject: [PATCH 6/6] Fix links in docs --- src/ast/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 989be2880..391532887 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -646,7 +646,7 @@ pub enum Expr { regexp: bool, }, /// `ANY` operation e.g. `foo > ANY(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` - /// https://docs.snowflake.com/en/sql-reference/operators-subquery#all-any + /// AnyOp { left: Box, compare_op: BinaryOperator, @@ -655,7 +655,7 @@ pub enum Expr { is_some: bool, }, /// `ALL` operation e.g. `foo > ALL(bar)`, comparison operator is one of `[=, >, <, =>, =<, !=]` - /// https://docs.snowflake.com/en/sql-reference/operators-subquery#all-any + /// AllOp { left: Box, compare_op: BinaryOperator,