Honour operator precedence in IS [NOT] DISTINCT FROM - #2436
Open
zvonimir-dd wants to merge 2 commits into
Open
Conversation
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 <noreply@anthropic.com>
IS [NOT] DISTINCT FROM right-operand precedenceIS [NOT] DISTINCT FROM
LucaCappelletti94
suggested changes
Aug 8, 2026
LucaCappelletti94
left a comment
Contributor
There was a problem hiding this comment.
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")
);
}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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parser::parse_infixparsed the right operand ofIS DISTINCT FROMandIS NOT DISTINCT FROMwithparse_expr, which isparse_subexpr(0), so the operand swallowed every following operator —including
ANDandOR.For instance,
a IS DISTINCT FROM 1 AND b = 2parsed asa IS DISTINCT FROM (1 AND b = 2)insteadof
(a IS DISTINCT FROM 1) AND (b = 2):PostgreSQL's operator precedence table places the
ISfamily aboveNOT,ANDandOR, soANDcannot be part of the right operand. Here is an expression that is well-typed under bothreadings and distinguishes them:
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 anIStoken — 17 in the defaultDialectimpl,IS_PRECinthe PostgreSQL dialect. Since
parse_subexprbreaks onprecedence >= next_precedence, the operandnow stops at
AND(10) andOR(5), and at a followingIS— which is what makes the familyassociate left — while still absorbing tighter operators such as
+(30). TheISarm is notdialect-gated, so this applies to every dialect; that looks intended, since MySQL's
<=>andMSSQL's
IS [NOT] DISTINCT FROMbind the same way.This is the same root cause as #2419, in a different hook. As there, nothing errored before, and
Displayadds no parentheses, so the wrong tree reprinted as the original text — which is why around trip never caught it and the new test asserts on the tree instead.
The new
parse_is_distinct_from_precedencecovers:Each of the first six produces an
IsDistinctFromat the root before this change; the last twoguard against over-tightening. The
IS NULLcase is theIS-family left-associativity symptom ofthe same precedence-0 call.
cargo test,cargo fmtandcargo clippyall pass.