apache / apache/datafusion-sqlparser-rs

MySQL `->` / `->>` bind too loosely against arithmetic and bitwise operators

Ouverte
#2,462 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Rust
Étoiles
3.5k
Forks
772
Merge moyen
4 j 9 h
PR mergées (30 j)
17

Description

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.

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Piste de recherche

Commencez par lire la Precedence definition et la gestion de la précédence de MySqlDialect, puis comparez PostgreSqlDialect::prec_value avec les assertions existantes dans tests/sqlparser_mysql.rs:879-910. Le travail est terminé lorsque les expressions fléchées MySQL sont regroupées au-dessus des opérateurs arithmétiques et bit à bit, que le comportement de PostgreSQL reste inchangé et que les tests au niveau de l’arbre couvrent les deux côtés de l’opérateur.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
mysql, rust, sql
Domaine
compilers, databases
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Active
Clarté
Plutôt claire
Accessibilité débutants
48/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.