zvonimir-dd opened a new issue, #2460: URL: https://github.com/apache/datafusion-sqlparser-rs/issues/2460
`DIV` is announced at `Precedence::MulDivModOp` by `get_next_precedence`, but the MySQL and Spark dialects parse its right operand with `parse_expr()` (= `parse_subexpr(0)`), so the operand absorbs every following operator instead of stopping at `DIV`'s own precedence. | SQL | Parsed as | 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 systems 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. `Display` for `Expr::BinaryOp` emits no parentheses, so a mis-grouped tree round-trips back to the original SQL — `verified_expr` / `verified_stmt` cannot catch this, which is why it has gone unnoticed. Only a test that asserts on the tree will. ### Fix Thread the `precedence` argument through, exactly as `SqliteDialect::parse_infix` already does for `REGEXP` / `MATCH` / `GLOB` (#2419): ```diff --- a/src/dialect/mysql.rs +++ b/src/dialect/mysql.rs - _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 - _precedence: u8, + precedence: u8, - let right = Box::new(match parser.parse_expr() { + let right = Box::new(match parser.parse_subexpr(precedence) { ``` ### Red test ```rust #[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") ); } ``` Credit to @LucaCappelletti94, who spotted this and wrote both the patch and the test while reviewing #2436. I'll open a PR once #2436 lands, to keep the two precedence changes reviewable separately — happy for someone else to pick it up sooner. -- 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]
