Skip to content

fix(pretty-print): round-trip ALTER TABLE identity, collation and deferral clauses - #828

Merged
psteinroe merged 1 commit into
mainfrom
fix/pretty-print-alter-table-cmds
Oct 2, 2026
Merged

psteinroe merged 1 commit into
mainfrom
fix/pretty-print-alter-table-cmds

Conversation

@psteinroe

Copy link
Copy Markdown
Collaborator

Fix four formatter bugs in ALTER TABLE commands and table constraints where the output either failed to re-parse or silently changed the statement. All seven affected statements from the Postgres 15–17 regression suite (#824) now round-trip at widths 80 and 100.

Identity column options

ALTER COLUMN ... SET GENERATED ... SET INCREMENT ... was emitted with a single leading SET, which doesn't parse, and a bare RESTART became SET RESTART, which Postgres rejects. Each option now gets its own SET, except RESTART, which never takes one. When the options don't fit on one line they break one per line, indented under the column.

-- before
alter column a set generated by default increment by 2 start with 100 restart;
-- after
alter column a
  set generated by default
  set increment by 2
  set start with 100
  restart;

Dropped clauses

  • ALTER COLUMN ... TYPE ... COLLATE now keeps its collation, placed before USING as the grammar requires.
  • ALTER TYPE ... ADD ATTRIBUTE ... CASCADE now keeps CASCADE.
  • EXCLUDE constraints now keep DEFERRABLE / INITIALLY DEFERRED. The deferral clause is now one helper shared by PRIMARY KEY, UNIQUE and EXCLUDE. No existing snapshot changes.

Refs #824

@psteinroe psteinroe added the ready label Oct 2, 2026
@psteinroe
psteinroe merged commit d19dae0 into main Oct 2, 2026
9 checks passed
@psteinroe
psteinroe deleted the fix/pretty-print-alter-table-cmds branch October 2, 2026 10:20
psteinroe added a commit that referenced this pull request Oct 2, 2026
)

Commit Postgres' own regression SQL (`src/test/regress/sql`) for every
supported major version, together with the verdict Postgres gave each
statement, so tests can run against real-world SQL without the network
or a database.

**Fixtures**

`crates/pgls_postgres_regress/data/<major>/` holds the pinned tag
(`REL_15_19`, `REL_16_15`, `REL_17_11`, `REL_18_6`), the upstream files
verbatim, and one `line:col verdict` file per SQL file, where the
verdict is `accepted`, `rejected <sqlstate>` or `skipped`.
`collate.windows.win1252.sql` is left out because it is not UTF-8 and
only runs on Windows. The data is marked `linguist-generated`, so the
diff collapses.

**Loader and recorder**

The crate's loader preprocesses each file (blanking psql meta-commands
and `COPY` data, so line numbers are kept), splits it with
`pgls_statement_splitter` and attaches the recorded verdicts. It panics
with "fixtures out of date, run `just record-regress <major>`" when the
split no longer matches. `just record-regress <major> [tag]` fetches the
tag, starts the matching `postgres` image in Docker, checks the server
version and runs every statement in a fresh database per file. The
semantics are the same as the existing type-check regression harness: 5s
statement timeout, transaction control and client `COPY` skipped,
`raw_sql`, roles cleaned up after each file.

**First consumer: pretty-print round trip**

A new `pgls_pretty_print` test formats every statement Postgres 15–17
accepted (about 92k), at widths 80 and 100, and requires the output to
parse back to the same normalized AST. Versions newer than the parser
are skipped. The formatter bugs it found are fixed in #825, #826, #827,
#828, #829 and #830, so it passes without an allowlist.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant