Skip to content

Honour operator precedence in MySQL and Spark DIV - #2471

Open
zvonimir-dd wants to merge 1 commit into
apache:mainfrom
zvonimir-dd:fix-div-precedence
Open

Honour operator precedence in MySQL and Spark DIV#2471
zvonimir-dd wants to merge 1 commit into
apache:mainfrom
zvonimir-dd:fix-div-precedence

Conversation

@zvonimir-dd

Copy link
Copy Markdown
Contributor

Fixes #2460.

Dialect::get_next_precedence announces DIV at Precedence::MulDivModOp, but MySqlDialect::parse_infix
and SparkSqlDialect::parse_infix both ignored their precedence argument and parsed the right
operand with parse_expr() (= parse_subexpr(0)), so the operand absorbed every following operator:

SQL Before After / MySQL & Spark
7 DIV 2 + 1 7 DIV (2 + 1) = 2 (7 DIV 2) + 1 = 4
9 DIV 3 * 3 9 DIV (3 * 3) = 1 (9 DIV 3) * 3 = 9
a DIV 2 = 1 a DIV (2 = 1) (a DIV 2) = 1

Both engines place DIV with * and /: MySQL's
operator precedence table lists
*, /, DIV, %, MOD on one row, and Spark's SqlBaseParser.g4 has
operator=(ASTERISK | SLASH | PERCENT | DIV) in a single left-recursive rule.

The fix threads the caller's precedence through to parse_subexpr, exactly as
SqliteDialect::parse_infix already does for REGEXP / MATCH / GLOB (#2419). Since
parse_subexpr passes the upcoming operator's precedence into the dialect hook, the right operand
is parsed at MulDivModOp, which stops it before + and makes DIV left-associative against *,
/ and itself.

Display for Expr::BinaryOp emits no parentheses, so a mis-grouped tree round-trips back to the
original SQL — verified_stmt / verified_expr alone cannot catch this. The new tests therefore
assert on the tree: tests/sqlparser_mysql.rs::parse_div_precedence covers +, *, DIV against
itself, =, and explicit parentheses; tests/sqlparser_spark.rs::test_div_precedence mirrors the
first two for Spark. I confirmed both fail without the source change.

GenericDialect is deliberately not included — it has no DIV operator support, so it never
produces MyIntegerDivide.

Full suite green (1605 tests, up 2 from 1603), cargo fmt --check and
cargo clippy --all-targets --all-features -- -D warnings clean.

Thanks to @LucaCappelletti94, who spotted this while reviewing #2436 and wrote both the patch and the
headline test.

`get_next_precedence` announces `DIV` at `Precedence::MulDivModOp`, but both
dialects parsed its right operand with `parse_expr()` (= `parse_subexpr(0)`),
so the operand absorbed every following operator: `7 DIV 2 + 1` grouped as
`7 DIV (2 + 1)` = 2 where MySQL and Spark both give `(7 DIV 2) + 1` = 4.

Thread the caller's precedence through instead, as SqliteDialect already does
for REGEXP / MATCH / GLOB. `Display` emits no parentheses, so a mis-grouped
tree round-trips to the original SQL and the round-trip helpers could not
catch this; the new tests assert on the tree.

Fixes apache#2460

Environment: Datadog workspace

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DIV right operand is parsed at precedence 0 in the MySQL and Spark dialects

2 participants