Print a space between adjacent prefix unary operators - #2424
Conversation
`Expr::UnaryOp`'s `Display` writes `{op}{expr}` with no separator, so when
the operand is itself a `UnaryOp` the two operator glyphs are emitted glued
together and the output no longer round-trips:
SELECT ~ ~ 1 -> SELECT ~~1 -> "Expected: an expression, found: ~~"
`SELECT - -1` prints as `SELECT --1`, which is the same defect but only
breaks on dialects where `--` opens a line comment; MySQL requires
whitespace after `--` and reparses it unchanged.
The Postgres case is worse than a parse error, because `@@` is a distinct
operator: `SELECT @ @ 1` (abs of abs) prints as `SELECT @@1` and silently
reparses as `UnaryOperator::DoubleAt` applied to `1`.
Extend the existing "needs a space" condition so it also fires when the
operand is another `Expr::UnaryOp`. Operators already in that list and
non-unary operands are unaffected, so `-1` and `NOT a` are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LucaCappelletti94
left a comment
LucaCappelletti94
left a comment
There was a problem hiding this comment.
At this time, the current implementation fires a bit too broadly, including also for cases where it should not, such as:
SELECT ++a --> SELECT + +a
SELECT +@a --> SELECT + @a
SELECT !!~a --> SELECT !! ~a
SELECT -NOT a --> SELECT - NOT aI believe it may be desirable to have a more focused alternative.
The previous condition fired for any nested `Expr::UnaryOp`, which spaced
pairs that already round-trip unchanged: `++a`, `+@a`, `!!~a`, `-NOT a`.
The hazard is not "the operand is a unary op", it is that the outer glyph
and the operand's leading glyph concatenate into a different token. Which
pairs do that follows from the tokenizer:
- `-`, `~`, `|/` and `||/` end in `start_binop`, so on a dialect with
`is_custom_operator_part` (Postgres) they absorb any following operator
character; `--` also opens a line comment everywhere except MySQL.
- `!` merges only with `!` and `~` (`!!`, `!~`).
- `@` merges only with `@`, `-` and `?` (`@@`, `@-@`, `@-`, `@?`).
- `+` and `!!` are returned by `consume_and_return` and never merge.
- A keyword operand (`NOT`) starts with a letter and never merges.
Verified by rendering every (outer, inner) prefix-operator pair as a nested
`UnaryOp` and reparsing it on all 15 dialects in `all_dialects()`: all 476
representable combinations round-trip, and the only spaces added beyond what
some dialect requires are `- !a` and `~ !a`, where the Hive-only `!` operand
is unreachable on the dialects that would merge it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Good catch — narrowed it. A space is now inserted only where the operator's glyph and the operand's leading glyph would tokenize as a single token, so I swept every prefix-operator pair across all dialects before and after the change and the set of round-trip breaks is identical, so the narrowing costs no coverage. It does leave two cosmetic spaces, Your four cases are pinned in |
2 participants
Expr::UnaryOp'sDisplaywrites{op}{expr}with no separator, so a nested unary operand is printed glued to the operator:prints as
SELECT ~~1, which no longer reparses on any dialect (Expected: an expression, found: ~~).SELECT - -1prints asSELECT --1the same way; that one only breaks where--opens a line comment, so MySQL is unaffected.On PostgreSQL it changes meaning rather than failing, because
@@is its own operator:SELECT @ @ 1(abs of abs) prints asSELECT @@1, which reparses asUnaryOperator::DoubleAtapplied to1.The fix inserts a space only where the operator's glyph and the operand's leading glyph would tokenize as a single token. Pairs that cannot merge are left alone, so
++a,+@a,!!~aand-NOT aprint unchanged.Tests are
parse_nested_unary_opsandparse_nested_pg_unary_ops, which fail without the source change, plusparse_adjacent_unary_ops_that_do_not_combineandparse_adjacent_pg_unary_ops_that_do_not_combinepinning the pairs that must stay glued. TheAGENTS.mdpre-commit checks are clean locally.