zvonimir-dd opened a new issue, #2462: URL: https://github.com/apache/datafusion-sqlparser-rs/issues/2462
Split out of #2461, where this was raised as an open question. MySQL's `->` / `->>` bind more tightly than every arithmetic, shift and bitwise operator: the right-hand side is a quoted JSON path, and `col->path` is defined as equivalent to `JSON_EXTRACT(col, path)` ([docs](https://dev.mysql.com/doc/refman/8.4/en/json-search-functions.html)). sqlparser gives the arrow operators `Precedence::PgOther` (21), which sits *below* `+` / `-` / `*`, so the arrow under-binds on both sides: | SQL | sqlparser (`MySqlDialect`, `GenericDialect`) | MySQL | |---|---|---| | `c -> '$.a' + 1` | `c -> ('$.a' + 1)` | `(c -> '$.a') + 1` | | `c ->> '$.a' + 1` | `c ->> ('$.a' + 1)` | `(c ->> '$.a') + 1` | | `c -> '$.a' * 2` | `c -> ('$.a' * 2)` | `(c -> '$.a') * 2` | | `1 + c -> '$.a'` | `(1 + c) -> '$.a'` | `1 + (c -> '$.a')` | Comparisons are already correct, since `Eq` (20) is below `PgOther` (21): `c -> '$.a' = 1` → `((c -> '$.a') = 1)`. As in the sibling issues, `Display` for `Expr::BinaryOp` emits no parentheses, so a mis-grouped tree round-trips to the original SQL — `verified_expr` / `verified_stmt` cannot catch this, only a test asserting on the tree can. ### Why this is not a fix to the shared row `PgOther` = 21 is *correct* for PostgreSQL. In `gram.y` the arrow shares one left-associative level with `|` and generic operators, below `+` / `-`: ``` %left Op OPERATOR RIGHT_ARROW '|' ``` and `PostgreSqlDialect` behaves accordingly today (`a -> b + c` → `a -> (b + c)`). So the two engines genuinely disagree about where the arrow sits, and a single shared precedence row cannot be right for both. ### Possible approaches 1. Add a dedicated `Precedence` variant for the arrow operators, defaulting to the current `PgOther` value so PostgreSQL and every other dialect are unchanged, and have `MySqlDialect` place it above `MulDivModOp`. This is the narrowest option. 2. Give `MySqlDialect` its own `prec_value` the way `PostgreSqlDialect` has one. That duplicates the whole table, and would also move `@>`, `<@` and `CustomBinaryOperator`, which is probably not intended. I'd lean towards (1), but I don't want to presume — happy to implement whichever a maintainer prefers. Two open questions for whoever picks this up: - Should `GenericDialect` follow MySQL here? It currently produces the same grouping as MySQL, but Generic is a permissive superset, so this seems like a judgment call rather than a clear bug. - How high should the MySQL arrow sit exactly? Since the right operand is lexically a path string in real MySQL, anything above `MulDivModOp` gives correct results for valid input; placing it near `DoubleColon` would match the grammar most literally. No existing test pins the current grouping — the only MySQL arrow trees asserted are single-operator ones inside a `CAST` (`tests/sqlparser_mysql.rs:879-910`). Related: #2436, #2460, #2461. -- 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]
