zvonimir-dd opened a new pull request, #2471:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2471

   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](https://dev.mysql.com/doc/refman/8.4/en/operator-precedence.html) 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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to