From 54e8e4739458b287694b2a2294659cc23b117a20 Mon Sep 17 00:00:00 2001 From: "zvonimir.rakamaric" Date: Fri, 7 Aug 2026 20:00:44 +0000 Subject: [PATCH 1/3] Fix `IS [NOT] DISTINCT FROM` right-operand precedence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Keyword::IS` arm of `parse_infix` parsed the right operand of `IS [NOT] DISTINCT FROM` with `parse_expr()`, i.e. `parse_subexpr(0)`, so the operand swallowed every following operator including `AND` and `OR`: `a IS DISTINCT FROM 1 AND b = 2` parsed as `a IS DISTINCT FROM (1 AND b = 2)`. Parse it at `precedence` instead, matching every other infix branch in the same function. For an `IS` token that is `prec_value(Precedence::Is)`, so the operand now stops at `AND` and `OR`, and at a following `IS` — making the `IS` family associate left — while still absorbing tighter operators. Environment: Datadog workspace Co-Authored-By: Claude Opus 5 --- src/parser/mod.rs | 4 +- tests/sqlparser_common.rs | 90 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index b2b3f42bb..d4b31bbc8 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -4071,11 +4071,11 @@ impl<'a> Parser<'a> { } else if self.parse_keywords(&[Keyword::NOT, Keyword::UNKNOWN]) { Ok(Expr::IsNotUnknown(Box::new(expr))) } else if self.parse_keywords(&[Keyword::DISTINCT, Keyword::FROM]) { - let expr2 = self.parse_expr()?; + let expr2 = self.parse_subexpr(precedence)?; Ok(Expr::IsDistinctFrom(Box::new(expr), Box::new(expr2))) } else if self.parse_keywords(&[Keyword::NOT, Keyword::DISTINCT, Keyword::FROM]) { - let expr2 = self.parse_expr()?; + let expr2 = self.parse_subexpr(precedence)?; Ok(Expr::IsNotDistinctFrom(Box::new(expr), Box::new(expr2))) } else if self.parse_keyword(Keyword::JSON) { self.parse_is_json_predicate(expr, false) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 0800bc41f..d08e79f02 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -1984,6 +1984,96 @@ fn parse_is_not_distinct_from() { ); } +#[test] +fn parse_is_distinct_from_precedence() { + use self::Expr::*; + + // The right operand of `IS [NOT] DISTINCT FROM` binds tighter than `AND`/`OR`, + // so the boolean operator must end up at the root of the tree. + assert_eq!( + BinaryOp { + left: Box::new(IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Expr::value(number("1"))), + )), + op: BinaryOperator::And, + right: Box::new(BinaryOp { + left: Box::new(Identifier(Ident::new("b"))), + op: BinaryOperator::Eq, + right: Box::new(Expr::value(number("2"))), + }), + }, + verified_expr("a IS DISTINCT FROM 1 AND b = 2") + ); + + assert_eq!( + BinaryOp { + left: Box::new(IsNotDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Expr::value(number("1"))), + )), + op: BinaryOperator::Or, + right: Box::new(BinaryOp { + left: Box::new(Identifier(Ident::new("b"))), + op: BinaryOperator::Eq, + right: Box::new(Expr::value(number("2"))), + }), + }, + verified_expr("a IS NOT DISTINCT FROM 1 OR b = 2") + ); + + // `AND` binds tighter than `OR` within the surrounding expression. + assert_matches!( + verified_expr("a IS DISTINCT FROM 1 AND b OR c"), + BinaryOp { + op: BinaryOperator::Or, + .. + } + ); + assert_matches!( + verified_expr("a IS DISTINCT FROM 1 OR b AND c"), + BinaryOp { + op: BinaryOperator::Or, + .. + } + ); + + // Explicit parentheses still push the boolean expression into the right operand. + assert_eq!( + IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Nested(Box::new(BinaryOp { + left: Box::new(Expr::value(number("1"))), + op: BinaryOperator::And, + right: Box::new(Identifier(Ident::new("b"))), + }))), + ), + verified_expr("a IS DISTINCT FROM (1 AND b)") + ); + + // The `IS` family is left-associative. + assert_eq!( + IsNull(Box::new(IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Identifier(Ident::new("b"))), + ))), + verified_expr("a IS DISTINCT FROM b IS NULL") + ); + + // Operators that bind tighter than `IS` are still part of the right operand. + assert_eq!( + IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(BinaryOp { + left: Box::new(Identifier(Ident::new("b"))), + op: BinaryOperator::Plus, + right: Box::new(Expr::value(number("1"))), + }), + ), + verified_expr("a IS DISTINCT FROM b + 1") + ); +} + #[test] fn parse_not_precedence() { // NOT has higher precedence than OR/AND, so the following must parse as (NOT true) OR true From 4c673489155f8c8422dbc694ef275635de4dbef7 Mon Sep 17 00:00:00 2001 From: "zvonimir.rakamaric" Date: Mon, 10 Aug 2026 16:16:03 +0000 Subject: [PATCH 2/3] Fix `PgOther` precedence in the default table Review of the `IS [NOT] DISTINCT FROM` fix surfaced that the default `Dialect::prec_value` places `Precedence::PgOther` (16) below `Is` (17), `Like` (19), `Eq` (20) and `Between` (20). PostgreSQL puts its "any other operator" class above all four, and the PostgreSQL dialect already agrees (`PG_OTHER_PREC` 70 vs `IS_PREC` 40), so the default table was the outlier. Parsing the `IS [NOT] DISTINCT FROM` right operand at its caller`s precedence exposed this as a regression for `->` and `@>` in the non-PostgreSQL dialects: `a IS DISTINCT FROM b -> k` began parsing as `(a IS DISTINCT FROM b) -> k`. Raise `PgOther` to 21, alongside `Pipe` and `Colon` (which map to `PG_OTHER_PREC` in the PostgreSQL dialect), so it sits above `Between`, `Eq`, `Like` and `Is`. Besides the reported regression this also repairs pre-existing mis-parses that were not caused by the previous commit: `a -> k = 1` parsed as `a -> (k = 1)` and `a @> b IS NULL` as `a @> (b IS NULL)`. Also sharpen the comment on the IS-family associativity case to record that the left-associative reading is deliberately more permissive than PostgreSQL, which declares IS as %nonassoc. Environment: Datadog workspace Co-Authored-By: Claude Opus 5 --- src/dialect/mod.rs | 2 +- tests/sqlparser_common.rs | 4 +++- tests/sqlparser_mysql.rs | 45 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index f99cbe2ea..43b343cbd 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -990,11 +990,11 @@ pub trait Dialect: Debug + Any { Precedence::Caret => 22, Precedence::Pipe => 21, Precedence::Colon => 21, + Precedence::PgOther => 21, Precedence::Between => 20, Precedence::Eq => 20, Precedence::Like => 19, Precedence::Is => 17, - Precedence::PgOther => 16, Precedence::UnaryNot => 15, Precedence::And => 10, Precedence::Or => 5, diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index d08e79f02..71048b4f9 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -2051,7 +2051,9 @@ fn parse_is_distinct_from_precedence() { verified_expr("a IS DISTINCT FROM (1 AND b)") ); - // The `IS` family is left-associative. + // sqlparser resolves the IS family left-associatively, consistent with how + // `a IS NULL IS NULL` already parses. Deliberately more permissive than + // PostgreSQL, which declares IS as %nonassoc and rejects the chain. assert_eq!( IsNull(Box::new(IsDistinctFrom( Box::new(Identifier(Ident::new("a"))), diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index 797a12551..e351a590d 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -4946,3 +4946,48 @@ fn parse_adjacent_string_literal_concatenation() { fn parse_group_by_with_rollup() { mysql().verified_stmt("SELECT * FROM tbl GROUP BY col1, col2 WITH ROLLUP"); } + +#[test] +fn parse_is_distinct_from_json_arrow_precedence() { + // MySQL's `->` binds tighter than `IS [NOT] DISTINCT FROM`, so the JSON + // extraction must stay inside the right operand. + assert_eq!( + Expr::IsDistinctFrom( + Box::new(Expr::Identifier(Ident::new("a"))), + Box::new(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("b"))), + op: BinaryOperator::Arrow, + right: Box::new(Expr::Value( + Value::SingleQuotedString("k".into()).with_empty_span() + )), + }), + ), + mysql().verified_expr("a IS DISTINCT FROM b -> 'k'") + ); +} + +#[test] +fn parse_json_arrow_comparison_precedence() { + // The same "any other operator" class also binds tighter than the + // comparison operators and `LIKE`, so the JSON extraction is the left + // operand rather than swallowing the right-hand side. + assert_eq!( + Expr::BinaryOp { + left: Box::new(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("a"))), + op: BinaryOperator::Arrow, + right: Box::new(Expr::Value( + Value::SingleQuotedString("k".into()).with_empty_span() + )), + }), + op: BinaryOperator::Eq, + right: Box::new(Expr::value(number("1"))), + }, + mysql().verified_expr("a -> 'k' = 1") + ); + + assert_matches!( + mysql().verified_expr("a -> 'k' LIKE 'x'"), + Expr::Like { .. } + ); +} From ebee69250b6fd67ff23c04bce040cf589631b073 Mon Sep 17 00:00:00 2001 From: Zvonimir Date: Wed, 26 Aug 2026 18:31:32 +0000 Subject: [PATCH 3/3] Address review feedback on precedence fix Document why `PgOther` sits above `Is` and why the `IS DISTINCT FROM` right operand is parsed at the caller's precedence. Tighten the loose root-only assertions into full-tree comparisons, and cover `NOT`, `=` as the right operand, and two chained `IS NOT DISTINCT FROM`. Move the `->` comparison-precedence coverage out of the MySQL suite into a cross-dialect test, and add `->>` to the MySQL arrow test. Environment: Datadog workspace Co-Authored-By: Claude Opus 5 --- src/dialect/mod.rs | 5 ++ src/parser/mod.rs | 3 + tests/sqlparser_common.rs | 115 +++++++++++++++++++++++++++++++++++--- tests/sqlparser_mysql.rs | 28 +++------- 4 files changed, 123 insertions(+), 28 deletions(-) diff --git a/src/dialect/mod.rs b/src/dialect/mod.rs index 43b343cbd..ff83a4da6 100644 --- a/src/dialect/mod.rs +++ b/src/dialect/mod.rs @@ -990,6 +990,11 @@ pub trait Dialect: Debug + Any { Precedence::Caret => 22, Precedence::Pipe => 21, Precedence::Colon => 21, + // "any other operator" -- `->`, `@>`, custom operators. PostgreSQL + // places this row above `BETWEEN` / `LIKE` and below `+` / `-` + // (`%left Op OPERATOR RIGHT_ARROW '|'` in gram.y), so it must bind + // more tightly than `IS`, whose right operand would otherwise stop + // short of it. Precedence::PgOther => 21, Precedence::Between => 20, Precedence::Eq => 20, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index d4b31bbc8..b7a532130 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -4071,6 +4071,9 @@ impl<'a> Parser<'a> { } else if self.parse_keywords(&[Keyword::NOT, Keyword::UNKNOWN]) { Ok(Expr::IsNotUnknown(Box::new(expr))) } else if self.parse_keywords(&[Keyword::DISTINCT, Keyword::FROM]) { + // The right operand binds no more loosely than `IS` + // itself, so that e.g. `a IS DISTINCT FROM b AND c` + // parses as `(a IS DISTINCT FROM b) AND c`. let expr2 = self.parse_subexpr(precedence)?; Ok(Expr::IsDistinctFrom(Box::new(expr), Box::new(expr2))) } else if self.parse_keywords(&[Keyword::NOT, Keyword::DISTINCT, Keyword::FROM]) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 71048b4f9..284a27aa8 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -2023,19 +2023,35 @@ fn parse_is_distinct_from_precedence() { ); // `AND` binds tighter than `OR` within the surrounding expression. - assert_matches!( - verified_expr("a IS DISTINCT FROM 1 AND b OR c"), + assert_eq!( BinaryOp { + left: Box::new(BinaryOp { + left: Box::new(IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Expr::value(number("1"))), + )), + op: BinaryOperator::And, + right: Box::new(Identifier(Ident::new("b"))), + }), op: BinaryOperator::Or, - .. - } + right: Box::new(Identifier(Ident::new("c"))), + }, + verified_expr("a IS DISTINCT FROM 1 AND b OR c") ); - assert_matches!( - verified_expr("a IS DISTINCT FROM 1 OR b AND c"), + assert_eq!( BinaryOp { + left: Box::new(IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Expr::value(number("1"))), + )), op: BinaryOperator::Or, - .. - } + right: Box::new(BinaryOp { + left: Box::new(Identifier(Ident::new("b"))), + op: BinaryOperator::And, + right: Box::new(Identifier(Ident::new("c"))), + }), + }, + verified_expr("a IS DISTINCT FROM 1 OR b AND c") ); // Explicit parentheses still push the boolean expression into the right operand. @@ -2074,6 +2090,89 @@ fn parse_is_distinct_from_precedence() { ), verified_expr("a IS DISTINCT FROM b + 1") ); + + assert_eq!( + IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(BinaryOp { + left: Box::new(Identifier(Ident::new("b"))), + op: BinaryOperator::Eq, + right: Box::new(Identifier(Ident::new("c"))), + }), + ), + verified_expr("a IS DISTINCT FROM b = c") + ); + + // `NOT` binds more loosely than `IS`, so it applies to the whole comparison. + assert_eq!( + UnaryOp { + op: UnaryOperator::Not, + expr: Box::new(IsDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Identifier(Ident::new("b"))), + )), + }, + verified_expr("NOT a IS DISTINCT FROM b") + ); + + assert_eq!( + BinaryOp { + left: Box::new(IsNotDistinctFrom( + Box::new(Identifier(Ident::new("a"))), + Box::new(Identifier(Ident::new("b"))), + )), + op: BinaryOperator::And, + right: Box::new(IsNotDistinctFrom( + Box::new(Identifier(Ident::new("c"))), + Box::new(Identifier(Ident::new("d"))), + )), + }, + verified_expr("a IS NOT DISTINCT FROM b AND c IS NOT DISTINCT FROM d") + ); +} + +#[test] +fn parse_pg_other_operator_precedence() { + let arrow_k = |left: Expr| Expr::BinaryOp { + left: Box::new(left), + op: BinaryOperator::Arrow, + right: Box::new(Expr::Value( + Value::SingleQuotedString("k".into()).with_empty_span(), + )), + }; + let t_a = || Expr::CompoundIdentifier(vec![Ident::new("t"), Ident::new("a")]); + + // `->` binds tighter than comparison operators, so the arrow expression is + // the left operand rather than the comparison being the arrow's key. + let expected_eq = |left: Expr| Expr::BinaryOp { + left: Box::new(arrow_k(left)), + op: BinaryOperator::Eq, + right: Box::new(Identifier(Ident::new("b"))), + }; + + // Dialects with lambda functions read a bare `a ->` as the start of a lambda. + assert_eq!( + expected_eq(Identifier(Ident::new("a"))), + all_dialects_where(|d| !d.supports_lambda_functions()).verified_expr("a -> 'k' = b") + ); + assert_eq!( + expected_eq(t_a()), + all_dialects().verified_expr("t.a -> 'k' = b") + ); + + // `LIKE` sits below `=` in the precedence table, so cover that boundary too. + assert_eq!( + Expr::Like { + negated: false, + any: false, + expr: Box::new(arrow_k(t_a())), + pattern: Box::new(Expr::Value( + Value::SingleQuotedString("x".into()).with_empty_span(), + )), + escape_char: None, + }, + all_dialects().verified_expr("t.a -> 'k' LIKE 'x'") + ); } #[test] diff --git a/tests/sqlparser_mysql.rs b/tests/sqlparser_mysql.rs index e351a590d..84e92836f 100644 --- a/tests/sqlparser_mysql.rs +++ b/tests/sqlparser_mysql.rs @@ -4962,32 +4962,20 @@ fn parse_is_distinct_from_json_arrow_precedence() { )), }), ), - mysql().verified_expr("a IS DISTINCT FROM b -> 'k'") + mysql_and_generic().verified_expr("a IS DISTINCT FROM b -> 'k'") ); -} -#[test] -fn parse_json_arrow_comparison_precedence() { - // The same "any other operator" class also binds tighter than the - // comparison operators and `LIKE`, so the JSON extraction is the left - // operand rather than swallowing the right-hand side. assert_eq!( - Expr::BinaryOp { - left: Box::new(Expr::BinaryOp { - left: Box::new(Expr::Identifier(Ident::new("a"))), - op: BinaryOperator::Arrow, + Expr::IsNotDistinctFrom( + Box::new(Expr::Identifier(Ident::new("a"))), + Box::new(Expr::BinaryOp { + left: Box::new(Expr::Identifier(Ident::new("b"))), + op: BinaryOperator::LongArrow, right: Box::new(Expr::Value( Value::SingleQuotedString("k".into()).with_empty_span() )), }), - op: BinaryOperator::Eq, - right: Box::new(Expr::value(number("1"))), - }, - mysql().verified_expr("a -> 'k' = 1") - ); - - assert_matches!( - mysql().verified_expr("a -> 'k' LIKE 'x'"), - Expr::Like { .. } + ), + mysql_and_generic().verified_expr("a IS NOT DISTINCT FROM b ->> 'k'") ); }