Repository navigation
test: add Postgres regression fixtures for every supported version - #824
Merged
Merged
Conversation
This was referenced Oct 2, 2026
psteinroe
added a commit
that referenced
this pull request
Oct 2, 2026
The formatter dropped `OVERRIDING USER VALUE` / `OVERRIDING SYSTEM VALUE` from the `INSERT` action of a `WHEN NOT MATCHED` clause, so the output re-parsed with `override = OverridingNotSet` and lost the identity-column semantics of the statement. `emit_merge_when_clause` now emits the overriding clause between the optional column list and `VALUES`, the same way `InsertStmt` does. This fixes the 4 MERGE statements from Postgres' `identity.sql` regression tests (15–17) that the corpus round-trip in #824 flagged. The new `merge_stmt_1` fixture covers both kinds, with and without a column list. No existing snapshots change. Refs #824
psteinroe
added a commit
that referenced
this pull request
Oct 2, 2026
… correctly (#826) Fix the `ALTER TEXT SEARCH CONFIGURATION` emitter so every mapping form re-parses to the same AST. Before this change, three of the forms produced SQL that failed to parse or parsed to a different statement. **Mapping forms** - `ALTER MAPPING REPLACE old WITH new` lost its `ALTER MAPPING REPLACE` keywords and printed `with old, new`, which is a syntax error. - `ALTER MAPPING FOR tokens REPLACE old WITH new` put `replace` after the dictionary list, which is also a syntax error. - `DROP MAPPING IF EXISTS` dropped `IF EXISTS`, so `missing_ok` changed from true to false. **Qualified dictionary names** Each dictionary is a `List` of name parts. The emitter printed those parts comma-separated, so `WITH public.english_stem` came out as `with public, english_stem`, which parses as two dictionaries. Dictionary names are now dot-joined. New fixtures `alter_tsconfiguration_stmt_1`–`4` cover these cases. They include the `tsdicts.sql` statements found by the regression corpus in #824. No existing snapshots changed.
psteinroe
added a commit
that referenced
this pull request
Oct 2, 2026
…ENAME (#827) Qualified object names in `ALTER ... SET SCHEMA` and `ALTER ... RENAME` no longer print with commas, and `ALTER TYPE ... RENAME ATTRIBUTE` keeps its type name and `CASCADE`. Before this, the formatted output for these statements did not re-parse. ```sql -- before alter domain alter1, posint set schema alter2; alter type rename attribute a to aa; -- after alter domain alter1.posint set schema alter2; alter type test_type2 rename attribute a to aa cascade; ``` **Qualified names** The parser stores `any_name` objects (types, domains, conversions, text search objects, collations, statistics) as a list of strings. The generic list emitter joined them with commas; they are now dot-separated, the same way `alter_owner_stmt` already handles them. In `RenameStmt` this also fixes qualified names in `RENAME TO` and domain `RENAME CONSTRAINT`. Neither appears in the Postgres regression corpus, but both have the same cause. **RENAME ATTRIBUTE** The composite type name lives in `relation`, not `object`, so it was dropped. It is now emitted without `ONLY` (the RangeVar has `inh = false`), and `CASCADE` is kept. Fixes the ALTER object-name group of the round-trip failures found in #824. The new fixtures only add snapshots; no existing snapshot changes.
psteinroe
added a commit
that referenced
this pull request
Oct 2, 2026
…erral clauses (#828) 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. ```sql -- 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
added a commit
that referenced
this pull request
Oct 2, 2026
The formatter dropped the parentheses that Postgres requires around a composite field access or a re-subscripted expression, which changed what the statement means: ```sql -- input -- output before this PR select (r).column2 from ss; select r.column2 from ss; -- column "column2" of table r order by (home_base[0])[0]; order by home_base[0][0]; -- 2-D subscript of home_base set d1.r = (d1).r - 1 set d1."r" = d1.r - 1 ``` `emit_a_indirection` flattened nested `AIndirection` nodes and only parenthesized a fixed list of base node types. It now emits each `AIndirection` as parsed. The base is wrapped in parentheses unless it is a parameter, a column reference followed by a subscript, or a scalar subquery, which brings its own parentheses. These are the only bases the parser accepts without them. **Normalizer no longer hides the mismatch** `normalize_a_indirection` merged `AIndirection(ColumnRef r, [f])` into `ColumnRef r.f` and flattened nested subscripts before the round-trip comparison. That treated semantically different trees as equal, so the bug only showed up under parents the function didn't walk into: `EXPLAIN`, views and CTEs, which is where the regression corpus in #824 caught it. The function is removed, along with the `BoolExpr` flattening that only it called (`normalize_bool_expr_associativity` already covers that for the whole tree). The same comparison runs in `pgls_workspace`'s format safety check, so formatting there becomes stricter, as intended. **Snapshot changes** 15 existing multi-statement snapshots change. Every change restores parentheses that are in the input, e.g. `(d1).r`, `(value).x` in domain checks, `(p.hobbies).equipment.name`, `(f1[1])[1]`. The exception is `subselect`, where `((select array[1, 2, 3]))[1]` becomes `(select array[1, 2, 3])[1]`; both parse to the same tree. This fixes the 11 composite-indirection statements from the Postgres 15–17 regression corpus. New fixtures: `a_indirection_0`–`2` and `update_stmt_1`. Refs #824
psteinroe
added a commit
that referenced
this pull request
Oct 2, 2026
) `CREATE FUNCTION` with a `BEGIN ATOMIC` body or a `TRANSFORM FOR TYPE` clause no longer formats to SQL that fails to re-parse. Both come from the Postgres 15–17 regression corpus in #824 (`create_function_sql.sql:177` and `object_address.sql:49/50`). **BEGIN ATOMIC** The body is a single-item list wrapping the statement list, and the generic list emitter joined the statements with commas (`select 1;, select false;`). Each statement now goes on its own line. An empty body no longer leaves a whitespace-only line, and `BEGIN ATOMIC` starts on its own line like the other clauses: ```sql create function functest_s_13() returns boolean begin atomic select 1; select false; end; ``` **TRANSFORM FOR TYPE** The option fell through to the generic emitter and came out as `TRANSFORM int`. It now emits `transform for type int, for type text` and sorts right after `LANGUAGE`, the same order pg_dump uses. **RETURN bodies** `RETURN` bodies went through the same code path and ended in a double semicolon (`immutable return 0;;`). They now end in a single semicolon, with `RETURN` on its own line. The `brin`, `btree_index`, `create_function_sql`, `create_procedure`, `test_setup` and `window` snapshots change only for these lines. Refs #824
psteinroe
force-pushed
the
test/postgres-regress-fixtures
branch
from
October 2, 2026 10:28
a0d9765 to
296e264
Compare
This was referenced Oct 2, 2026
psteinroe
added a commit
that referenced
this pull request
Oct 4, 2026
Restructures the linter around flat rule IDs and replaces the EXPLAIN-based typecheck with static type checking backed by an in-memory catalog. The linter infers the type of every expression and reports what Postgres would reject, without running the statement. EXPLAIN stays as a fallback. Existing configs and suppression comments keep working and print deprecation warnings. Completions and hover now see the tables and columns created earlier in the file. This combines the former stack of #815, #819 and #823 into one PR. **Flat rule IDs and groups** Rules are identified by name: `lint/banDropColumn`, `linter.rules.banDropColumn`, `pgls-ignore banDropColumn`. Groups are metadata, configured in bulk under `linter.groups`, like oxlint's categories. Moving a rule to another group no longer breaks configs or suppressions. The rules were regrouped into `correctness`, `safety`, `destructive`, `style`, `typecheck`, and `nursery`. ```jsonc // before { "linter": { "rules": { "safety": { "banDropColumn": "off" } } } } // after { "linter": { "rules": { "banDropColumn": "off" }, "groups": { "style": "off" } } } ``` Precedence is rule > deprecated `linter.rules.safety.<rule>` > group > `recommended`/`all` preset. `preferBigintOverInt` and `preferBigintOverSmallint` are merged into `preferBigInt` with the options `checkInt` and `checkSmallint`. `concurrentRefreshMatviewLock` is removed; `requireConcurrentRefreshMatview` covers it, and the old name maps to it. **Backward compatibility and warnings** Everything users write keeps working: - **Config:** `linter.rules.safety.<rule>`, `safety.recommended`, and `safety.all` still apply, including the removed rule names, which map to `preferBigInt`. The CLI prints one `Warning:` line per deprecated setting before it runs. The LSP shows a single warning message per session that lists them. The JSON schema marks `linter.rules.safety` as `deprecated`, so editors flag it in the config file. - **Suppressions:** `lint/safety/<rule>`, `lint/<group>`, and the removed rule names still suppress; a removed rule stands for the rule that replaced it. Each such comment gets a warning diagnostic that names the flat form, like ``Use `banDropColumn` instead``. `lint/safety` keeps its former meaning of every lint rule. `safety` is today's group. - **Rule selectors:** the `only` and `skip` lists of the workspace API accept the old IDs and the removed rule names. `safety` and `lint/safety` still select every lint rule. - **API:** The generated TypeScript types keep their names, including `Safety` and `SchemaCache`. So do the JSON formats, the `schema export` command, and the `invalidateSchemaCache` command. What changes with an unchanged setup: - **Diagnostic categories:** `lint/safety/<rule>` becomes `lint/<rule>`. Scripts that match on the LSP diagnostic code or the JSON, GitHub, GitLab, or JUnit reporter output need updating. - **`linter.enabled: false`** now disables the linter. It used to be ignored. - **`migrations.migrationsDir`:** when set, migration-only rules (most of `safety` and `destructive`) are skipped for files outside it. **Catalog and name resolution** The new crate `pgls_catalog` merges `pgls_schema_cache` and adds: - a catalog that applies the file's DDL to the database snapshot: temp tables, CTAS and view columns, inheritance, renames, drops, and `ROLLBACK` / savepoints; - the session state: search path, role, transactions, locks, `check_function_bodies`; - a conservative name resolver. No DDL runs against the database. Anything the catalog can't be sure about stays silent, including statements that don't parse and may have changed the catalog. EXPLAIN remains as a fallback for statements that only reference unchanged database objects. `typecheck.enabled` switches both off. The unused `pgls_type_resolver` crate is deleted. **Typecheck rules** All are recommended and report errors. | Rule | Reports | Postgres error | | --- | --- | --- | | `unknownRelation`, `unknownColumn`, `unknownSchema`, `unknownFunction`, `unknownType` | references to objects that don't exist | 42P01, 42703, 3F000, 42883, 42704 | | `ambiguousColumn` | a column of several FROM items | 42702 | | `insertColumnMismatch` | more values than target columns, or the other way round | 42601 | | `missingFromClauseEntry` | `t.col` without `t` in FROM | 42P01 | | `operatorTypeMismatch` | `1 + now()`, `'1' + '1'` | 42883, 42725 | | `functionArgumentMismatch` | `length(1)`, ambiguous overloads | 42883, 42725 | | `invalidCast` | `now()::int` | 42846 | | `assignmentTypeMismatch` | `insert into t (qty) values (now())`, also `UPDATE` and `ON CONFLICT` | 42804 | | `functionReturnTypeMismatch` | `LANGUAGE sql` functions whose final statement returns the wrong number or types of columns | 42P13 | **How typing works** The snapshot now loads casts, operators, and typing metadata for types and functions. `pgls_catalog::typing` ports the algorithms of the latest supported Postgres (18, pinned to `REL_18_6`) for coercion, common types, polymorphic types, and function and operator overload selection. Each port links the Postgres function it follows at that tag. Postgres 15 to 17 resolve types the same way; where 18 accepts more (`old`/`new` in `RETURNING`), it is gated on the server version. The resolver types every clause Postgres types. Anything it doesn't model is unknown, and unknown never reports. `AGENTS.md` describes how to port and test the type rules. **No false positives** Two tests compare the analysis with Postgres: - `resolve/differential_tests.rs` checks a corpus of statements against a live Postgres on every run. Statements Postgres accepts must give no findings, inferred column types must match what Postgres describes, and expected errors must be found. - `pgls_analyser/tests/postgres_regress.rs` runs Postgres' own regression SQL for 15, 16, 17 and 18 (the fixtures from #824, about 170,000 statements) through the linter, against the catalog of a fresh database on each version. It is a normal `cargo test` without network or database and takes about 11 seconds. Any finding on a statement Postgres accepted fails it. The findings on statements Postgres rejected are snapshotted per version, so changes in detection show up in review. `just record-regress <major> --catalog-only` re-records a catalog. It found 644 false positives on Postgres 15 and 426 more on 16 to 18; all are fixed. One of them affected the editor beyond typing: statements that don't parse were dropped before linting, so a `CREATE TABLE` with newer syntax made every later use of the table report a missing relation. Such statements now make missing objects unknown for the rest of the file, unless they can't change the catalog (queries, `COPY`, maintenance, ...). **Schema cache** The `SchemaCache` JSON gains casts, operators, and typing fields on types and functions. All of them are optional, so older exports still load. Review focus: false positives in `pgls_catalog/src/resolve`, `catalog/ddl` and `typing`. The known gaps are listed in the module docs of `catalog/ddl`, `resolve` and `typing`. Statements with psql identifier parameters (`:schema.table`) are not typechecked. Fixes #624 Fixes #692 Fixes #369 Fixes #431
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
1 participant
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.
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 oneline:col verdictfile per SQL file, where the verdict isaccepted,rejected <sqlstate>orskipped.collate.windows.win1252.sqlis left out because it is not UTF-8 and only runs on Windows. The data is markedlinguist-generated, so the diff collapses.Loader and recorder
The crate's loader preprocesses each file (blanking psql meta-commands and
COPYdata, so line numbers are kept), splits it withpgls_statement_splitterand attaches the recorded verdicts. It panics with "fixtures out of date, runjust record-regress <major>" when the split no longer matches.just record-regress <major> [tag]fetches the tag, starts the matchingpostgresimage 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 clientCOPYskipped,raw_sql, roles cleaned up after each file.First consumer: pretty-print round trip
A new
pgls_pretty_printtest 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.