zvonimir-dd commented on issue #2461:
URL:
https://github.com/apache/datafusion-sqlparser-rs/issues/2461#issuecomment-5480782388
I started on the one-line fix proposed above and hit a complication worth
settling before there's a PR to review: `&` sits in a ladder, so moving it
changes its relation to every neighbour, not just to `->`. Two other groupings
flip, and one of them contradicts real MySQL and BigQuery. That turns this into
a question about what the default table is *for*, which seems like a maintainer
call rather than something to decide inside a PR.
### What actually moves
The relevant slice of `Dialect::prec_value` (`src/dialect/mod.rs`), tightest
first. Note that `Precedence::Caret` covers `<<` and `>>` as well as `^` and
`#`:
| Precedence | Value | Tokens |
|---|---|---|
| `PlusMinus` | 30 | `+` `-` |
| `Xor` | 24 | `XOR` keyword |
| `Ampersand` | 23 → **21** | `&` |
| `Caret` | 22 | `^` `#` `<<` `>>` |
| `Pipe` / `Colon` / `PgOther` | 21 | `\|`, `:`, `->` `->>` `@>`
`OPERATOR(…)` |
| `Between` / `Eq` | 20 | `=` `<` `>` … |
With `Ampersand => 21`, three groupings change:
| Expression | Before | After | Relation that moved |
|---|---|---|---|
| `t.a -> 'k' & b` | `t.a -> ('k' & b)` | `(t.a -> 'k') & b` | the reported
bug — `23 > 21` becomes a tie |
| `a \| b & c` | `a \| (b & c)` | `(a \| b) & c` | `&` no longer outranks
`\|` |
| `a & b ^ c` | `(a & b) ^ c` | `a & (b ^ c)` | `Caret` (22) now outranks
`&` (21) |
### How that lands against the engines
- **PostgreSQL** — `&`, `|`, `<<`, `>>` and `->` are a single
left-associative row in `gram.y` (`%left Op OPERATOR RIGHT_ARROW '|'`), so rows
1 and 2 become PG-correct. `PostgreSqlDialect` overrides `prec_value` and is
unaffected either way; the argument here is only that the default table
describes itself as approximating PG, and `postgresql.rs` shows what that row
is meant to look like.
- **MySQL** — the documented order is `^` > `*` `/` `DIV` `%` > `+` `-` >
`<<` `>>` > `&` > `|`. Row 3 becomes *correct* for MySQL, an improvement. Row 2
becomes *wrong*: MySQL reads `a | b & c` as `a | (b & c)`.
- **BigQuery** — C-style `&` > `^` > `|`, which is exactly what the table
has today. This is the one dialect where both rows 2 and 3 are pure regressions.
- **SQLite** — `& | << >>` are one level, so row 2 gets closer and row 3
stays wrong in the opposite direction. SQLite also puts `->`/`->>` *above* `*`
and `/`, so row 1 is not right for SQLite before or after.
- **T-SQL** — `+ - & ^ |` are all one level, so rows 2 and 3 are
approximations either way.
Two things bound the blast radius:
- `Display` for `Expr::BinaryOp` emits no parentheses, so a regrouped tree
still round-trips to byte-identical SQL. Only AST consumers can observe any of
this — which is also why no `verified_stmt` test caught the original divergence.
- The full test suite passes with `Ampersand => 21` applied, so nothing
in-tree currently pins any of the three groupings.
I also tried the other direction — tying `PgOther` up to 23 to meet `&`
rather than moving `&` down. It fixes row 1, but breaks the `|`/`->` tie that
is currently PG-correct and shifts `->` against `^`/`<<`/`>>`, so it trades one
divergence for two. I don't think a zero-collateral variant exists.
### The question
1. **`Ampersand => 21` alone** — accept the `&`/`|` tie and the `&`/`^`
reversal as the price of matching the table's stated PostgreSQL basis. One line
plus tests.
2. **`Ampersand => 21` plus `prec_value` overrides on `MySqlDialect` and
`BigQueryDialect`** that keep `& > ^ > |` (and, for MySQL, `<<`/`>>` above
`&`). Nothing regresses, but it touches three precedence tables and reopens
MySQL's ladder more broadly — `XOR` is currently at 24, where MySQL puts it
between `AND` and `OR`, and `<<`/`>>` are fused with `Caret`.
3. **Close this as intended behaviour** — treat the bitwise block as
deliberately C-style and accept that `->` binds looser than `&` outside
PostgreSQL.
Happy to implement whichever you prefer. Worth noting that the `->`-vs-`&`
case only bites in dialects that use `->` as an operator, which overlaps with
#2462 (MySQL `->`/`->>` binding looser than all arithmetic), so it may be
easier to settle the two together.
--
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]
