-
Notifications
You must be signed in to change notification settings - Fork 158
Permalink
Choose a base ref
{{ refName }}
default
Choose a head ref
{{ refName }}
default
Checking mergeability…
Don’t worry, you can still create the pull request.
Comparing changes
Choose two branches to see what’s changed or to start a new pull request.
If you need to, you can also or
learn more about diff comparisons.
Open a pull request
Create a new pull request by comparing changes across two branches. If you need to, you can also .
Learn more about diff comparisons here.
base repository: DBDiff/DBDiff
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: fix/partitioned-table-renderer
Could not load branches
Nothing to show
Loading
Could not load tags
Nothing to show
{{ refName }}
default
Loading
...
head repository: DBDiff/DBDiff
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: master
Could not load branches
Nothing to show
Loading
Could not load tags
Nothing to show
{{ refName }}
default
Loading
- 3 commits
- 2 files changed
- 3 contributors
Commits on Sep 15, 2026
-
fix(postgres): keep a partitioned table's primary key inline (#212)
## The bug `pg_dump` writes a partitioned parent's primary key as ```sql ALTER TABLE ONLY public.readings ADD CONSTRAINT readings_pkey PRIMARY KEY (taken_on, id); ``` `ONLY` means the index reaches the partitions that exist **at the moment it runs** and no others. In `pg_dump`'s own output order the key precedes every `CREATE TABLE ... PARTITION OF`, so they inherit it and the migration is correct. Reorder the statements and it silently isn't. Grouping a fix set by object kind — every table created before any constraint is added — is a reasonable way to apply a migration, and SupaForge's `orderStatements` does exactly that (`CREATE_TABLE` phase 40 before `ALTER_TABLE` phase 50). The partitions are then created before the key is added, never receive it, and the migration reports success having dropped it. **This reached 3.0.0-rc.10.** Every partitioned table with a primary key diffed by that release, on a machine with `pg_dump`, loses its partition keys when applied by a consumer that reorders. It surfaced as a SupaForge e2e failure whose fingerprint showed the partitions missing `readings_2025_pkey` / `readings_2026_pkey`, their indexes, and the `inh` attach links. Introduced by `4e9c912` ("render DDL with pg_dump when it is available"), which put `PgDumpRenderer::tableDDL()` first in `getCreateStatement` for **every** table. rc.9 was immune because the built-in renderer emits the key inline. ## The fix Partition metadata is read before `pg_dump` is tried, so partitioned tables use the built-in renderer and get the key inside `CREATE TABLE`, where the statement order cannot matter. The real fault is DDL whose correctness depends on the order it happens to be written in. This is also what the README already claimed — "Partitions always use the built-in renderer" — which stopped being true when `pg_dump` rendering began taking precedence. ## Why the suite missed it The corpus **does** cover partitioned tables with primary keys (`obj_partitioned_range`, `obj_partition_index`, `hard_part_index_propagated`), and those cases passed throughout. The conformance runner applies DBDiff's statements in DBDiff's own order — the one order in which this bug is invisible. Nothing asserted that the emitted DDL survives being reordered. So that is what the new test does. `PartitionedTableRendererPostgresTest` has two cases, both verified to fail without this change: - the parent's key is inline and the DDL contains no `ALTER TABLE ONLY` - the statements are replayed with every `CREATE TABLE` moved first, and each partition must still have its primary key — without the fix this fails with `readings_2025 lost its primary key` The second encodes the property rather than the symptom, and is the one that would have caught the original regression. I also considered and **disproved** two other explanations before finding this: that the stale `pg_dump` archive cache (#211) was hiding it, and that single-transaction application in the runner was masking it. Both were tested; neither holds. ## Cost One corpus case: `hard_part_expr_key`, an expression partition key that `pg_dump` reproduced and the built-in renderer does not. It was already in `known-failures.json`, so the baseline is unchanged and the suite reports no new failures. Measured back to back, twice each way, to be sure of the delta. | PostgreSQL 16, 90-case corpus | before | after | |---|---|---| | built-in | 68 / 90 | 68 / 90 | | with `pg_dump` | 82 / 90 | **81 / 90** | The README's figure and the reason for the exception are updated. ## Test plan - [x] New test fails without the fix (both cases), passes with it - [x] Verified the fix is targeted, not a blanket disable: an ordinary table still renders via `pg_dump`, a partitioned parent does not - [x] Full conformance suite, both renderer modes: `Failed (no diff): 0`, **zero new failures** (304 with `pg_dump`, 291 built-in) - [x] 782 unit, 68 Postgres, 16 SQLite pass - [x] Corpus delta measured back to back in one session rather than from separate runs, after an initial single measurement proved to be an artifact of leftover scratch databases 🤖 Generated with [Claude Code](https://claude.com/claude-code)Configuration menu - View commit details
-
Copy full SHA for dd4a88c - Browse repository at this point
Copy the full SHA dd4a88cView commit details -
fix(postgres): reproduce a serial column's sequence ownership
A serial column's sequence belongs to that column. PostgreSQL records the link as a dependency, drops the sequence when the column goes, and a schema reader uses it to tell an owned sequence from a standalone one. pg_dump writes the link as its own table-of-contents entry, SEQUENCE OWNED BY, and wantsType did not ask for it. The SEQUENCE and DEFAULT entries alone reproduce what the column does but not what it owns, so a table copied by DBDiff held a sequence owned by nothing. Nothing looked at sequence ownership until sequences became a modelled object kind, so the copy being subtly wrong did not show. Once they were modelled, a diff between the original and the copy found a standalone sequence on one side only and generated a DROP for it — which PostgreSQL refuses, because the column default still depends on it: cannot drop sequence shipments_id_seq because other objects depend on it The migration then rolls back entirely, so a schema DBDiff had itself created could not be diffed again. Reached 3.0.0-rc.10 and rc.11. Asserted as a round trip rather than as text, because the text was never the point: reproduce the table into an empty database, and the sequence has to be as invisible to the object-kind reader on the copy as it is on the original. The test fails without this change. TABLE DATA and SEQUENCE SET stay excluded — this renders schema, and a schema diff neither copies rows nor moves a sequence's current value. Verified through SupaForge, whose suite is where this surfaced: its lifecycle e2e file goes from 31 of 32 to 32 of 32, and its full suite to 1225 passing. Conformance is unchanged at 304 with pg_dump and 291 without, no new failures on either path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Configuration menu - View commit details
-
Copy full SHA for 1fbd6c7 - Browse repository at this point
Copy the full SHA 1fbd6c7View commit details -
fix(postgres): reproduce a serial column's sequence ownership (#213)
## The bug A `serial` column's sequence belongs to that column. PostgreSQL records the link as a dependency, drops the sequence when the column goes, and a schema reader uses it to tell an owned sequence from a standalone one. `pg_dump` writes that link as its own table-of-contents entry — `SEQUENCE OWNED BY` — and `wantsType` never asked for it: ``` 216; 1259 TABLE public shipments 215; 1259 SEQUENCE public shipments_id_seq 3423; 0 0 SEQUENCE OWNED BY public shipments_id_seq <-- filtered out 3267; 2604 DEFAULT public shipments id ``` `SEQUENCE` and `DEFAULT` reproduce what the column *does* but not what it *owns*, so a table copied by DBDiff held a sequence owned by nothing. Measured directly: | | `shipments_id_seq` ownership | |---|---| | original | `a` (owned by the column) | | DBDiff's copy | **`UNOWNED`** | Nothing read sequence ownership until sequences became a modelled object kind in #209, so the copy being subtly wrong never showed. Once they were modelled, a diff between the original and the copy found a standalone sequence on one side only and generated a `DROP` for it — which PostgreSQL refuses, because the column default still depends on it: ``` 1 error(s): ✗ [schema] schema-drop-sequence-4: cannot drop sequence shipments_id_seq because other objects depend on it Nothing was written. ``` The migration rolls back entirely, so **a schema DBDiff had itself created could not be diffed again**. This reached rc.10 and rc.11. ## The fix Add `SEQUENCE OWNED BY` to the allowlist. It is part of a table's own DDL in exactly the way `DEFAULT` is. `TABLE DATA` and `SEQUENCE SET` stay excluded: this renders schema, and a schema diff neither copies rows nor moves a sequence's current value. ## Why it escaped `PostgresObjectKindsTest` covers serial, identity and standalone sequences — but only on a **freshly created** schema, never on one DBDiff had itself reproduced. The defect needs a second diff against DBDiff's own output to appear, and nothing asserted that round trip. That is the same shape as #212: both bugs are invisible in a single pass and only surface on the second. So the test asserts the round trip rather than the text — reproduce the table into an empty database, and the sequence must be as invisible to the object-kind reader on the copy as it is on the original. It fails without this change. ## Verification Found and confirmed through SupaForge's own suite, where it surfaced. Its e2e lifecycle file is order-dependent in a useful way: the failure needs an earlier test to have synced a table with a serial column first, which is why the case passes in isolation and fails in the file. | DBDiff | whole `lifecycle.test.ts` | |---|---| | rc.9 | 32 / 32 | | rc.10, rc.11 | 31 pass, 1 fail | | **this branch** | **32 / 32** | Run against this branch's source via a shim over the released binary, with the rc.11 pins otherwise untouched so only the DBDiff code differed. SupaForge's full suite: **1225 passing, 3 skipped, 59/59 files**. - [x] New test fails without the fix, passes with it - [x] 782 unit, 69 Postgres, 16 SQLite pass - [x] Conformance unchanged — 304 with `pg_dump`, 291 without, `Failed (no diff): 0`, no new failures on either path ## Note on the release rc.11 is currently the `latest` dist-tag and carries this, so anything installing `@dbdiff/cli` today hits it when re-diffing a schema containing a `serial` column. Worth an rc.12 once this lands. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Configuration menu - View commit details
-
Copy full SHA for d99f75f - Browse repository at this point
Copy the full SHA d99f75fView commit details
Loading
This comparison is taking too long to generate.
Unfortunately it looks like we can’t render this comparison for you right now. It might be too big, or there might be something weird with your repository.
You can try running this command locally to see the comparison on your machine:
git diff fix/partitioned-table-renderer...master