Repository navigation
fix(pretty_print): keep with admin option portable - #900
Merged
Merged
Conversation
…functions break sensibly
psteinroe
changed the base branch from
fix/pretty-print-function-call-groups
to
main
October 26, 2026 08:01
psteinroe
added a commit
that referenced
this pull request
Oct 26, 2026
Stacked on #900. Postgres keeps the first 63 bytes of an identifier (`NAMEDATALEN - 1`), and so does the AST the formatter works from. A longer name was therefore rewritten to its truncated form: ```sql grant select on t to a_role_name_that_is_definitely_way_too_long_for_anything_postgres_keeps; -- before grant select on table t to a_role_name_that_is_definitely_way_too_long_for_anything_postgr; -- after grant select on table t to a_role_name_that_is_definitely_way_too_long_for_anything_postgres_keeps; ``` A quoted name could even end up cut in the middle, with a space before the closing quote. Postgres truncates both spellings the same way, so the meaning didn't change, but `docs/features/formatting.md` says the formatter doesn't change what is written. **What changed** `format_statement` now scans the statement for identifier tokens longer than 63 bytes, and maps the name the parser cut each one down to back to the name as written: dequoted for a quoted identifier, lower-cased for an unquoted one. The truncation clips to whole characters like `truncate_identifier`, so a multibyte character at the cut is handled. The identifier emitters (`emit_identifier`, `emit_identifier_maybe_quoted`, `emit_type_identifier_maybe_quoted`) look names up there and write the full name, quoted where needed as before. The AST check still passes, because the output parses back to the same truncated names. A name is only restored when that is unambiguous. If two spellings are cut to the same 63 bytes, or a name is written both in full and already truncated, the AST name is kept as is. **Tests** - Unit tests in `identifiers.rs`: unquoted and quoted names, a multibyte character at the cut, and the two ambiguous cases. - `identifiers_longer_than_postgres_keeps_are_written_in_full` in `source_fidelity.rs` covers a role, a quoted column and a table. No snapshot changes. The snapshot harness drives the emitter directly without the source, and no fixture has an identifier this long. **Not included: deeply nested statements (finding P22)** About 40 nested CASEs, 20 nested subqueries or 45 levels of AND/OR still fail with "recursion limit reached". This limit is prost's decode limit, a constant 100 in prost 0.13. The only way to change it is the `no-recursion-limit` feature, which removes the limit for every prost user in the build. Without a limit, hostile input would hit stack overflows, which abort the process, in the decoder and in the recursive AST walkers (formatter, `iter`, normalisation) instead of returning an error. Doing that safely needs a bounded decoder or parsing on a thread with a larger stack. That isn't a small change, so I left it out of this PR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #898.
grant r1 to u1 with admin optionwas rewritten towith admin true. Both spellings parse to the same AST since Postgres 16, but Postgres 15 and older only acceptwith admin option, so formatting broke the statement for them:The GRANT role option emitter now writes
admin optionwhen the admin option is true.admin false,inherit true|falseandset true|falsekeep the boolean spelling: they only exist since Postgres 16, which also accepts it. A trueadminmixed with them prints aswith inherit false, admin option, which Postgres 16 accepts too.I ran the output against the local Postgres 15.19:
with admin optionandwith admin option granted by current_rolework, andwith admin trueis a syntax error there.Snapshot changes
All reviewed; one category:
with admin true→with admin option(privileges, both widths).Test:
grant_role_with_admin_option_keeps_the_portable_spellinginsource_fidelity.rscoversadmin option,admin trueand the Postgres 16 options.