Honour operator precedence in IS [NOT] DISTINCT FROM

[zvonimir-dd]

Parser::parse_infix parsed the right operand of IS DISTINCT FROM and IS NOT DISTINCT FROM with
parse_expr, which is parse_subexpr(0), so the operand swallowed every following operator —
including AND and OR.

For instance, a IS DISTINCT FROM 1 AND b = 2 parsed as a IS DISTINCT FROM (1 AND b = 2) instead
of (a IS DISTINCT FROM 1) AND (b = 2):

IsDistinctFrom(
    Identifier("a"),
    BinaryOp { left: Number("1"), op: And, right: BinaryOp { left: b, op: Eq, right: 2 } },
)

PostgreSQL's operator precedence table places the IS family above NOT, AND and OR, so
AND cannot be part of the right operand. Here is an expression that is well-typed under both
readings and distinguishes them:

SELECT true IS DISTINCT FROM true AND false;
-- (true IS DISTINCT FROM true) AND false  =  false AND false             =  false  <- correct
--  true IS DISTINCT FROM (true AND false) =  true IS DISTINCT FROM false =  true   <- before this PR

DuckDB, which follows PostgreSQL precedence here, returns false.

These two branches were ignoring the precedence their caller passed; using it fixes both. That value
is prec_value(Precedence::Is) for an IS token — 17 in the default Dialect impl, IS_PREC in
the PostgreSQL dialect. Since parse_subexpr breaks on precedence >= next_precedence, the operand
now stops at AND (10) and OR (5), and at a following IS — which is what makes the family
associate left — while still absorbing tighter operators such as + (30). The IS arm is not
dialect-gated, so this applies to every dialect; that looks intended, since MySQL's <=> and
MSSQL's IS [NOT] DISTINCT FROM bind the same way.

This is the same root cause as #2419, in a different hook. As there, nothing errored before, and
Display adds no parentheses, so the wrong tree reprinted as the original text — which is why a
round trip never caught it and the new test asserts on the tree instead.

The new parse_is_distinct_from_precedence covers:

a IS DISTINCT FROM 1 AND b = 2       ->  (a IS DISTINCT FROM 1) AND (b = 2)
a IS NOT DISTINCT FROM 1 OR b = 2    ->  (a IS NOT DISTINCT FROM 1) OR (b = 2)
a IS DISTINCT FROM 1 AND b OR c      ->  ((a IS DISTINCT FROM 1) AND b) OR c
a IS DISTINCT FROM 1 OR b AND c      ->  (a IS DISTINCT FROM 1) OR (b AND c)
a IS DISTINCT FROM (1 AND b)         ->  unchanged (explicit parens)
a IS DISTINCT FROM b IS NULL         ->  (a IS DISTINCT FROM b) IS NULL
a IS DISTINCT FROM b + 1             ->  a IS DISTINCT FROM (b + 1)

Each of the first six produces an IsDistinctFrom at the root before this change; the last two
guard against over-tightening. The IS NULL case is the IS-family left-associativity symptom of
the same precedence-0 call.

cargo test, cargo fmt and cargo clippy all pass.

[LucaCappelletti94]

While reading through the PR, I noticed that also DIV has the identical defect. 7 DIV 2 + 1 → 7 DIV (2 + 1) = 2, while MySQL gives 4. It is only vaguely associated to the current PR in terms of precedence errors, so it should likely be a different subsequent PR, I just wanted to jot it down so as to not forget it.

--- a/src/dialect/mysql.rs
+++ b/src/dialect/mysql.rs
@@ -99,10 +99,10 @@ impl Dialect for MySqlDialect {
-        _precedence: u8,
+        precedence: u8,
-            let right = Box::new(match parser.parse_expr() {
+            let right = Box::new(match parser.parse_subexpr(precedence) {

--- a/src/dialect/spark.rs
+++ b/src/dialect/spark.rs
@@ -138,9 +138,9 @@ impl Dialect for SparkSqlDialect {
-        _precedence: u8,
+        precedence: u8,
-            let right = Box::new(match parser.parse_expr() {
+            let right = Box::new(match parser.parse_subexpr(precedence) {

A red test for this could be:

#[test]
fn parse_div_precedence() {
    // `DIV` has the same precedence as `*` and `/`, so `+` must end up at the root.
    assert_eq!(
        Expr::BinaryOp {
            left: Box::new(Expr::BinaryOp {
                left: Box::new(Expr::value(number("7"))),
                op: BinaryOperator::MyIntegerDivide,
                right: Box::new(Expr::value(number("2"))),
            }),
            op: BinaryOperator::Plus,
            right: Box::new(Expr::value(number("1"))),
        },
        mysql().verified_expr("7 DIV 2 + 1")
    );
}

[LucaCappelletti94]

This regresses -> / @> in the non-PostgreSQL dialects, such as MySQL. I suggest you add the following red test in tests/sqlparser_mysql.rs, and proceed from there.

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

The issue to be clear is found in the default table, while your patch merely exposes this, but then it is a good occasion to fix it.

[LucaCappelletti94]

Suggested change

	// 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.