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 <noreply@anthropic.com>

Author: zvonimir-dd

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,

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])

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]

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'")
     );
 }