Refactoring: Merge the IN expression variants into Expr::In

src/ast/mod.rs

@@ -871,6 +871,30 @@ impl fmt::Display for CaseWhen {
     }
 }
 
+/// The right-hand side of an `IN` predicate, see [`Expr::In`].
+#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
+#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
+#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
+pub enum InTarget {
+    /// `(val1, val2, ...)`
+    List(Vec<Expr>),
+    /// `(SELECT ...)`
+    Subquery(Box<Query>),
+    /// `UNNEST(array_expression)`, see
+    /// [BigQuery](https://cloud.google.com/bigquery/docs/reference/standard-sql/operators#in_operators)
+    Unnest(Box<Expr>),
+}
+
+impl fmt::Display for InTarget {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            InTarget::List(list) => write!(f, "({})", display_comma_separated(list)),
+            InTarget::Subquery(subquery) => write!(f, "({subquery})"),
+            InTarget::Unnest(array_expr) => write!(f, "UNNEST({array_expr})"),
+        }
+    }
+}
+
 /// An SQL expression of any type.
 ///
 /// # Semantics / Type Checking
@@ -975,37 +999,17 @@ pub enum Expr {
         /// `true` when `NOT` is present.
         negated: bool,
     },
-    /// `[ GLOBAL ] [ NOT ] IN (val1, val2, ...)`
-    InList {
+    /// `<expr> [ GLOBAL ] [ NOT ] IN <target>`
+    In {
         /// Left-hand expression to test for membership.
         expr: Box<Expr>,
-        /// Literal list of expressions to check against.
-        list: Vec<Expr>,
+        /// What `expr` is tested against.
+        target: InTarget,
         /// `true` when the `NOT` modifier is present.
         negated: bool,
         /// `true` when the ClickHouse `GLOBAL` modifier is present.
         global: bool,
     },
-    /// `[ GLOBAL ] [ NOT ] IN (SELECT ...)`
-    InSubquery {
-        /// Left-hand expression to test for membership.
-        expr: Box<Expr>,
-        /// The subquery providing the candidate values.
-        subquery: Box<Query>,
-        /// `true` when the `NOT` modifier is present.
-        negated: bool,
-        /// `true` when the ClickHouse `GLOBAL` modifier is present.
-        global: bool,
-    },
-    /// `[ NOT ] IN UNNEST(array_expression)`
-    InUnnest {
-        /// Left-hand expression to test for membership.
-        expr: Box<Expr>,
-        /// Array expression being unnested.
-        array_expr: Box<Expr>,
-        /// `true` when the `NOT` modifier is present.
-        negated: bool,
-    },
     /// `<expr> [ NOT ] BETWEEN <low> AND <high>`
     Between {
         /// Expression being compared.
@@ -1801,42 +1805,18 @@ impl fmt::Display for Expr {
                 }
                 Ok(())
             }
-            Expr::InList {
+            Expr::In {
                 expr,
-                list,
-                negated,
-                global,
-            } => write!(
-                f,
-                "{} {}{}IN ({})",
-                expr,
-                if *global { "GLOBAL " } else { "" },
-                if *negated { "NOT " } else { "" },
-                display_comma_separated(list)
-            ),
-            Expr::InSubquery {
-                expr,
-                subquery,
+                target,
                 negated,
                 global,
             } => write!(
                 f,
-                "{} {}{}IN ({})",
+                "{} {}{}IN {}",
                 expr,
                 if *global { "GLOBAL " } else { "" },
                 if *negated { "NOT " } else { "" },
-                subquery
-            ),
-            Expr::InUnnest {
-                expr,
-                array_expr,
-                negated,
-            } => write!(
-                f,
-                "{} {}IN UNNEST({})",
-                expr,
-                if *negated { "NOT " } else { "" },
-                array_expr
+                target
             ),
             Expr::Between {
                 expr,

src/ast/spans.rs

@@ -36,9 +36,9 @@ use super::{
     CreateTableOptions, Cte, Delete, DoUpdate, ExceptSelectItem, ExcludeConstraintElement,
     ExcludeSelectItem, Expr, ExprWithAlias, Fetch, ForValues, FromTable, Function, FunctionArg,
     FunctionArgExpr, FunctionArgumentClause, FunctionArgumentList, FunctionArguments, GroupByExpr,
-    HavingBound, IfStatement, IlikeSelectItem, IndexColumn, Insert, Interpolate, InterpolateExpr,
-    Join, JoinConstraint, JoinOperator, JsonPath, JsonPathElem, LateralView, LimitClause,
-    MatchRecognizePattern, Measure, Merge, MergeAction, MergeClause, MergeInsertExpr,
+    HavingBound, IfStatement, IlikeSelectItem, InTarget, IndexColumn, Insert, Interpolate,
+    InterpolateExpr, Join, JoinConstraint, JoinOperator, JsonPath, JsonPathElem, LateralView,
+    LimitClause, MatchRecognizePattern, Measure, Merge, MergeAction, MergeClause, MergeInsertExpr,
     MergeInsertKind, MergeUpdateExpr, MergeUpdateKind, NamedParenthesizedList,
     NamedWindowDefinition, ObjectName, ObjectNamePart, Offset, OnConflict, OnConflictAction,
     OnInsert, OpenStatement, OrderBy, OrderByExpr, OrderByKind, OutputClause, Parens, Partition,
@@ -1525,25 +1525,18 @@ impl Spanned for Expr {
             } => expr.span(),
             Expr::IsDistinctFrom(lhs, rhs) => lhs.span().union(&rhs.span()),
             Expr::IsNotDistinctFrom(lhs, rhs) => lhs.span().union(&rhs.span()),
-            Expr::InList {
+            Expr::In {
                 expr,
-                list,
+                target,
                 negated: _,
                 global: _,
-            } => union_spans(
-                core::iter::once(expr.span()).chain(list.iter().map(|item| item.span())),
-            ),
-            Expr::InSubquery {
-                expr,
-                subquery,
-                negated: _,
-                global: _,
-            } => expr.span().union(&subquery.span()),
-            Expr::InUnnest {
-                expr,
-                array_expr,
-                negated: _,
-            } => expr.span().union(&array_expr.span()),
+            } => match target {
+                InTarget::List(list) => union_spans(
+                    core::iter::once(expr.span()).chain(list.iter().map(|item| item.span())),
+                ),
+                InTarget::Subquery(subquery) => expr.span().union(&subquery.span()),
+                InTarget::Unnest(array_expr) => expr.span().union(&array_expr.span()),
+            },
             Expr::Between {
                 expr,
                 negated: _,
@@ -3249,4 +3242,20 @@ SETTINGS update_parallel_mode = 'auto'"#;
             stmt => panic!("expected ALTER TABLE; got {stmt:?}"),
         }
     }
+
+    #[test]
+    fn test_in_expression_spans() {
+        for (sql, expected) in [
+            ("  needle IN (first, last)", "needle IN (first, last"),
+            (
+                "  needle IN (SELECT last FROM items)",
+                "needle IN (SELECT last FROM items",
+            ),
+            ("  needle IN UNNEST(items)", "needle IN UNNEST(items"),
+        ] {
+            let mut test = SpanTest::new(&GenericDialect, sql);
+            let expr = test.0.parse_expr().unwrap();
+            assert_eq!(test.get_source(expr.span()), expected, "{sql}");
+        }
+    }
 }

src/parser/mod.rs

@@ -4476,26 +4476,30 @@ impl<'a> Parser<'a> {
     ) -> Result<Expr, ParserError> {
         // BigQuery allows `IN UNNEST(array_expression)`
         // https://cloud.google.com/bigquery/docs/reference/standard-sql/operators#in_operators
-        if !global && self.parse_keyword(Keyword::UNNEST) {
+        let target = if !global && self.parse_keyword(Keyword::UNNEST) {
             self.expect_token(&Token::LParen)?;
             let array_expr = self.parse_expr()?;
             self.expect_token(&Token::RParen)?;
-            return Ok(Expr::InUnnest {
-                expr: Box::new(expr),
-                array_expr: Box::new(array_expr),
-                negated,
-            });
-        }
-        if self.dialect.supports_in_unparenthesized_expr()
+            InTarget::Unnest(Box::new(array_expr))
+        } else if self.dialect.supports_in_unparenthesized_expr()
             && self.peek_token_ref().token != Token::LParen
         {
-            return Ok(Expr::InList {
-                expr: Box::new(expr),
-                list: vec![self.parse_subexpr(self.dialect.prec_value(Precedence::Eq))?],
-                negated,
-                global,
-            });
-        }
+            InTarget::List(vec![
+                self.parse_subexpr(self.dialect.prec_value(Precedence::Eq))?
+            ])
+        } else {
+            self.parse_in_parenthesized_target()?
+        };
+        Ok(Expr::In {
+            expr: Box::new(expr),
+            target,
+            negated,
+            global,
+        })
+    }
+
+    /// Parses the parenthesized subquery or value list of an `IN` predicate.
+    fn parse_in_parenthesized_target(&mut self) -> Result<InTarget, ParserError> {
         self.expect_token(&Token::LParen)?;
         // A leading query is the whole subquery only when `)` follows it,
         // otherwise it is the first item of a list, as in `IN ((SELECT 1), 2)`.
@@ -4508,12 +4512,7 @@ impl<'a> Parser<'a> {
                 let mut missing_rparen = None;
                 if let Some(subquery) = self.maybe_parse(|p| p.parse_query())? {
                     if self.consume_token(&Token::RParen) {
-                        return Ok(Expr::InSubquery {
-                            expr: Box::new(expr),
-                            subquery,
-                            negated,
-                            global,
-                        });
+                        return Ok(InTarget::Subquery(subquery));
                     }
                     missing_rparen = Some(self.index);
                     self.index = start;
@@ -4529,12 +4528,7 @@ impl<'a> Parser<'a> {
             self.parse_comma_separated(Parser::parse_expr)
         };
         match list.and_then(|list| self.expect_token(&Token::RParen).map(|_| list)) {
-            Ok(list) => Ok(Expr::InList {
-                expr: Box::new(expr),
-                list,
-                negated,
-                global,
-            }),
+            Ok(list) => Ok(InTarget::List(list)),
             Err(ParserError::RecursionLimitExceeded) => Err(ParserError::RecursionLimitExceeded),
             Err(e) => match missing_rparen {
                 Some(index) if self.index <= index => {

tests/sqlparser_clickhouse.rs

@@ -2120,12 +2120,12 @@ fn parse_global_in() {
     let select = clickhouse().verified_only_select("SELECT * FROM t WHERE x GLOBAL IN (1, 2)");
     assert_eq!(
         select.selection.unwrap(),
-        Expr::InList {
+        Expr::In {
             expr: Box::new(Identifier(Ident::new("x"))),
-            list: vec![
+            target: InTarget::List(vec![
                 Expr::Value(number("1").with_empty_span()),
                 Expr::Value(number("2").with_empty_span()),
-            ],
+            ]),
             negated: false,
             global: true,
         }
@@ -2135,9 +2135,9 @@ fn parse_global_in() {
         .verified_only_select("SELECT * FROM t WHERE x GLOBAL NOT IN (SELECT y FROM t2)");
     assert_eq!(
         select.selection.unwrap(),
-        Expr::InSubquery {
+        Expr::In {
             expr: Box::new(Identifier(Ident::new("x"))),
-            subquery: Box::new(clickhouse().verified_query("SELECT y FROM t2")),
+            target: InTarget::Subquery(Box::new(clickhouse().verified_query("SELECT y FROM t2"))),
             negated: true,
             global: true,
         }
@@ -2222,7 +2222,7 @@ fn global_in_bare_table_binds_before_and() {
             left,
             op: BinaryOperator::And,
             ..
-        } if matches!(*left, Expr::InList { global: true, .. })
+        } if matches!(*left, Expr::In { target: InTarget::List(_), global: true, .. })
     ));
 }
 
@@ -2235,7 +2235,7 @@ fn global_in_bare_table_binds_before_equality() {
             left,
             op: BinaryOperator::Eq,
             ..
-        } if matches!(*left, Expr::InList { global: true, .. })
+        } if matches!(*left, Expr::In { target: InTarget::List(_), global: true, .. })
     ));
 }