fix(pg-delta): recreate procedure expression dependents#291
Conversation
🦋 Changeset detectedLatest commit: b5bc112 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1841170f82
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfa9c03515
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e8685bfe2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8535da107
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a4075c5a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fde4809611
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77b4ea3d60
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a47a809c02
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3a87e29d2a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e719c5ea1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0bee0e7e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a32dcdd050
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c064a4c1e9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4510a36fde
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 79f9c9fbf9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
79f9c9f to
bc014e2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc014e2432
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eee437a39d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
a291d09 to
c041b26
Compare
|
Rebased this branch onto latest Validation run:
|
|
Follow-up validation rerun after Docker became available locally:
The branch is still at |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73d9e85fdd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
59a6de9 to
9da80c2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9da80c2247
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cae3add04e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d774f8eb98
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0070138992
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 134e83e8a2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Replay retained column grants for generated columns whose DROP/ADD recreation is already covered by the original diff, limited to the affected covered column.\n\nReplay retained comment and statistics metadata for constraint-backed indexes recreated by table constraint DDL, alongside the existing cluster restore. These metadata changes require the index stable id, so sorting keeps them after the constraint-backed index is recreated.\n\nRED evidence from de660ed:\n- covered generated-column recreation produced 0 retained GrantTablePrivileges.\n- retained constraint-backed index replay produced no CreateCommentOnIndex.\n\nGREEN validation:\n- focused unit regressions: 3 pass / 0 fail.\n- expand-replace-dependencies.test.ts: 65 pass / 0 fail.\n- PG15 covered generated-column integration case: 1 pass / 0 fail after retry.\n- format-and-lint:fix, check-types, knip --fix, and git diff --check passed.\n\nSigned-off-by: xiaominghao <xiaominghao@baidu.com>
RED evidence before the fix:\n\n- `bun run test src/core/expand-replace-dependencies.test.ts --test-name-pattern "owned sequence owners|targeted constraint replacement"` failed because the retained owned sequence replay emitted no `ALTER SEQUENCE ... OWNER TO app_owner` and targeted constraint replacement emitted no retained backing-index comment/statistics/cluster metadata.\n- `bun run test src/core/post-diff-normalization.test.ts --test-name-pattern "owner restores"` failed because both table owner restores remained instead of keeping only the replacement replay.
GREEN validation after the fix:\n\n- `bun run test src/core/expand-replace-dependencies.test.ts --test-name-pattern "owned sequence owners|targeted constraint replacement"` -> 2 pass / 0 fail.\n- `bun run test src/core/post-diff-normalization.test.ts --test-name-pattern "owner restores"` -> 1 pass / 0 fail.\n- Full related unit reruns: `expand-replace-dependencies.test.ts` -> 67 pass / 0 fail; post-diff normalization + sequence files -> 30 pass / 0 fail.\n- PG17 focused integration for constraint-backed generated-column index metadata -> 1 pass / 0 fail with 10s connect timeout override.
RED evidence before the fix: - defers promoted table owner restore: expected ownerIndex > addConstraintIndex; received ownerIndex=4, addConstraintIndex=5. - orders promoted table and owned sequence owner restores: expected tableOwnerIndex > addConstraintIndex; received tableOwnerIndex=8, addConstraintIndex=9. - traverses foreign keys that reference recreated column constraints: expected one item_refs_item_value_fkey drop, received 0. - PG17 retained unique generated column FK integration added for end-to-end replay ordering.
Fixes the PR supabase#291 review follow-ups by: - ordering table owner restores after table-local replay DDL and before owned-sequence attachment via explicit sort constraints; - ordering sequence owner restores before ALTER SEQUENCE ... OWNED BY; - continuing traversal from recreated constraints so FK dependents are also released/restored without duplicate constraint changes. Validation after the fix: - focused owner and FK unit regressions pass. - expand-replace-dependencies.test.ts: 70 pass / 0 fail. - related sort/post-diff/sequence unit subset: 37 pass / 0 fail after PG17-limited retry; initial unrestricted run failed in testcontainers PG15 healthcheck before assertions. - PG17 FK integration passed twice, with testcontainers retries (attempt 6, then attempt 4). - format-and-lint:fix, check-types, knip --fix, and git diff --check pass.
RED evidence before the fix: - focused materialized-view unit regression failed with ownerIndex=6 and clusterIndex=7, proving ALTER MATERIALIZED VIEW OWNER sorted before CLUSTER ON. - focused sort regression failed with sequenceOwnerIndex=0 and tableOwnerIndex=1, proving ALTER SEQUENCE OWNER sorted before the owning table OWNER change. - the removed domain CHECK constraint guard already passed on current code because AlterDomainDropConstraint exposes the constraint stable id through drops; it is kept as explicit coverage for the review thread.
Count AlterDomainDropConstraint as explicit release coverage for routine-dependent domain CHECK constraints. Add materialized-view owner-last custom ordering so retained CLUSTER ON replay runs before OWNER TO. Order owned sequence owner changes after the owning table owner change, while keeping existing OWNED BY attachment ordering after both owners. GREEN validation after the fix: - focused materialized-view and domain guard tests: 2 pass / 0 fail. - focused owned sequence owner sort test: 1 pass / 0 fail.
RED evidence before the fix: - sequence.diff focused regression failed with ownerIndex=0 and detachIndex=1, proving ALTER SEQUENCE OWNER was emitted before OWNED BY NONE for an existing owned sequence detach. - sort-changes focused regression failed with ownerIndex=1 and grantIndex=2, proving sequence owner restore sorted before sequence ACL replay. - PG17 integration coverage verifies generated SQL order for detach-before-owner and grant-before-owner, then applies the plan and checks final owners/ACL.
Detach existing owned sequences before changing owner when OWNED BY is being removed or moved, so PostgreSQL no longer sees the sequence attached to the old owning table during ALTER SEQUENCE OWNER. Add sequence owner-last sort constraints for same-sequence metadata and privilege replay, while preserving the final OWNED BY attachment after owner restoration. RED evidence from 5d2fb6c before the fix: - sequence.diff focused regression failed with ownerIndex=0 and detachIndex=1. - sort-changes focused regression failed with ownerIndex=1 and grantIndex=2. GREEN validation after the fix: - PG17 focused unit regressions: 2 pass / 0 fail. - related unit files: 29 pass / 0 fail. - PG17 sequence owner integration: 1 pass / 0 fail. - format-and-lint:fix, check-types, knip --fix, and git diff --check passed.
RED evidence before the fix:\n- focused expand-replace-dependencies regression failed with AlterProcedureChangeOwner length 2, expected 1.\n- focused domain regression failed with AlterDomainDropDefault length 0, expected 1.\n- focused owner ordering regression failed with ownerIndex=5 before securityLabelIndex=7.
De-duplicate routine alters superseded by dependency-expansion recreation, avoid treating domain constraint drops as domain default release coverage, and order routine owner restores after metadata and ACL replay.\n\nGREEN verification:\n- npx --yes bun@1.3.14 run test src/core/expand-replace-dependencies.test.ts --test-name-pattern "de-duplicates superseded owner alters|defers promoted routine owner restore|does not count domain constraint drops" -> 3 pass / 0 fail.\n- npx --yes bun@1.3.14 run test src/core/expand-replace-dependencies.test.ts -> 75 pass / 0 fail.\n- npx --yes bun@1.3.14 run test src/core/sort/sort-changes.test.ts -> 14 pass / 0 fail.\n- npx --yes bun@1.3.14 run format-and-lint:fix -> pass.\n- npx --yes bun@1.3.14 run check-types -> pass.\n- npx --yes bun@1.3.14 run knip --fix -> exit 0, existing configuration hints only.\n- git diff --check -> pass.
RED evidence before fix: - sequence.diff focused test failed with ownerIndex=-1 after CreateSequence. - expandReplaceDependencies focused test failed with createSequences length 0 for a sequence cascade-dropped by main-side OWNED BY. - trigger.alter focused test failed because SetTriggerEnabledState.requires only contained the trigger stable id.
RED evidence before the fix: - focused post-diff normalization regression failed because the normalized change list still contained the original AlterTableAddColumn for a table already promoted to DropTable/CreateTable. Signed-off-by: xiaominghao <xiaominghao@baidu.com>
Drop same-table AlterTableAddColumn changes when dependency expansion has promoted that table to DropTable/CreateTable. The replacement CreateTable already includes the branch columns, so keeping the original ADD COLUMN can fail with column already exists after table recreation. RED evidence from 4a74cff: - focused post-diff normalization regression failed because the normalized change list still contained the original AlterTableAddColumn for a replaced table. GREEN verification: - npx --yes bun@1.3.14 run test src/core/post-diff-normalization.test.ts --test-name-pattern "prunes same-table add-column" -> 1 pass / 0 fail. - npx --yes bun@1.3.14 run test src/core/post-diff-normalization.test.ts -> 12 pass / 0 fail. - npx --yes bun@1.3.14 run test src/core/expand-replace-dependencies.test.ts --test-name-pattern "promoted table" -> 10 pass / 0 fail. - npx --yes bun@1.3.14 run format-and-lint:fix, check-types, knip --fix, and git diff --check passed. Signed-off-by: xiaominghao <xiaominghao@baidu.com>
RED evidence before fix: - post-diff identity-add pruning: focused test kept AlterTableAlterColumnAddIdentity for the replaced table. - routine grant replay: focused test found 0 GrantProcedurePrivileges after the original grant alter was pruned. - promoted trigger cleanup: focused test kept the stale ReplaceTrigger alongside DropTrigger/CreateTrigger.
GREEN verification before commit: - npx --yes bun@1.3.14 run test src/core/post-diff-normalization.test.ts --test-name-pattern identity-add -> 1 pass / 0 fail. - npx --yes bun@1.3.14 run test src/core/expand-replace-dependencies.test.ts --test-name-pattern 'preserves new grants|prunes superseded trigger alters' -> 2 pass / 0 fail. - npx --yes bun@1.3.14 run test src/core/post-diff-normalization.test.ts -> 13 pass / 0 fail. - npx --yes bun@1.3.14 run test src/core/expand-replace-dependencies.test.ts -> 78 pass / 0 fail.
RED before the fix: - expandReplaceDependencies regular-to-generated grant replay expected 1 GrantTablePrivileges but received 0. - expandReplaceDependencies publication row-filter replay sorted AlterPublicationSetOwner at index 2 before AlterPublicationAddTables at index 3. - sortChanges view metadata replay sorted AlterViewChangeOwner before GrantViewPrivileges, owner index 2 vs grant index 3. - sortChanges publication table replay sorted AlterPublicationSetOwner before AlterPublicationAddTables, owner index 0 vs add index 1.
Fixes the latest PR supabase#291 review batch: - replay retained column grants for covered regular/generated status column recreations. - defer view owner restores until after retained view metadata and ACL replay. - defer publication owner restores until after replayed publication table membership. GREEN verification: - ./node_modules/.bin/bun run --cwd packages/pg-delta test src/core/expand-replace-dependencies.test.ts src/core/sort/sort-changes.test.ts -> 98 pass / 0 fail. - PGDELTA_SKIP_DUMMY_SECLABEL_BUILD=1 PGDELTA_TEST_POSTGRES_VERSIONS=17 TESTCONTAINERS_RYUK_DISABLED=true ./node_modules/.bin/bun run --cwd packages/pg-delta test tests/integration/function-signature-expression-dependencies.test.ts --test-name-pattern "regular-to-generated column recreation restores retained column grants|generated publication row filters without column lists" -> 2 pass / 0 fail. - ./node_modules/.bin/bun run format-and-lint:fix && ./node_modules/.bin/bun run check-types && ./node_modules/.bin/bun run knip --fix && git diff --check -> exit 0 (knip existing hints only).
RED evidence before the fix: - sortChanges view trigger replay failed with ownerIndex=2 before trigger commentIndex=3. - sortChanges view rule metadata replay failed with ownerIndex=2 before rule commentIndex=3. - expandReplaceDependencies promoted table replay kept two AlterSequenceChangeOwner changes, expected one. Signed-off-by: xiaominghao <xiaominghao@baidu.com>
Map view-owned trigger and rule replay changes back to the owning view for owner-last sort constraints, including trigger/rule metadata wrappers that otherwise only expose trigger: or rule: stable ids. Prune original AlterSequenceChangeOwner changes when promoted table replay recreates the same retained owned sequence and emits the replacement owner restore. RED evidence from 63c5bc2: - view trigger comment replay sorted after AlterViewChangeOwner. - view rule comment/enabled-state replay sorted after AlterViewChangeOwner. - promoted table sequence replay kept two AlterSequenceChangeOwner changes. GREEN verification: - ./node_modules/.bin/bun run --cwd packages/pg-delta test src/core/sort/sort-changes.test.ts --test-name-pattern "view trigger replay|view rule metadata" -> 2 pass / 0 fail. - ./node_modules/.bin/bun run --cwd packages/pg-delta test src/core/expand-replace-dependencies.test.ts --test-name-pattern "prunes original owned sequence owner alters" -> 1 pass / 0 fail. - ./node_modules/.bin/bun run --cwd packages/pg-delta test src/core/expand-replace-dependencies.test.ts src/core/sort/sort-changes.test.ts -> 101 pass / 0 fail. - PGDELTA_SKIP_DUMMY_SECLABEL_BUILD=1 PGDELTA_TEST_POSTGRES_VERSIONS=17 TESTCONTAINERS_RYUK_DISABLED=true ./node_modules/.bin/bun run --cwd packages/pg-delta test tests/integration/sequence-operations.test.ts --test-name-pattern "sequence owner restores run after detach and privilege replay" -> 1 pass / 0 fail. - ./node_modules/.bin/bun run format-and-lint:fix, check-types, knip --fix, and git diff --check passed. Signed-off-by: xiaominghao <xiaominghao@baidu.com>
7f0b0ae to
becb9ed
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: becb9edbdd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
RED evidence before this fix: - expandReplaceDependencies retained regular-to-generated comment test failed because DropTable was present: Expected false, Received true. - expandReplaceDependencies moved owned-sequence default test failed because set-default changes did not contain archived_items.archived_value: Received []. - sortChanges quoted-dot trigger metadata test failed because owner restore stayed before metadata: ownerIndex 0, commentIndex 1. - sortChanges domain expression replay test failed because owner restore stayed before domain replay: ownerIndex 0, setDefaultIndex 1.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b6320e878
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
RED evidence: - quoted-dot table stable id regression failed with AlterTableAlterColumnDropDefault length expected 1, received 0 - truncated generated NOT NULL constraint regression failed with DropTable expected false, received true
1 participant
Summary
Fixes two related
pg-deltareplacement-expansion bugs where procedure replacement walks throughpg_dependexpression edges and either over-recreates the owning object or silently skips a domain CHECK dependent.Fixes #280
Fixes #286
Root Cause
expandReplaceDependencies(...)seeds replacement roots from dropped/recreated procedure stable IDs, then walksmainCatalog.dependsto find dependents that must be handled before the old procedure can be dropped.For expression containers, the old behavior was too coarse:
column:*dependents were normalized to the owningtable:*, causingDropTable+CreateTableeven when a targeted column-default or generated-expression update is enough.constraint:*dependents were treated as table-owned by default. That over-recreated tables for table CHECK constraints and also skipped domain CHECK constraints by normalizingconstraint:schema.domain.connameto a nonexistenttable:schema.domain.domain:*default dependencies were resolved as whole-domain replacements, which could further cascade intoDropTablewhen a table column used the domain.Fix
This keeps the fix in the catalog/dependency path and does not add SQL parsing.
The replacement expander now treats procedure expression dependents as targeted release/restore units before falling back to owner replacement:
ALTER TABLE ... DROP DEFAULTbefore old procedure drop andALTER TABLE ... SET DEFAULT ...after new procedure creation;ALTER TABLE ... SET EXPRESSION AS ...only on PostgreSQL 17+, preserving the existing destructive fallback on older versions where in-place generated-expression updates are not available;ALTER TABLE ... DROP CONSTRAINT/ADD CONSTRAINT;ALTER DOMAIN ... DROP DEFAULT/SET DEFAULT;constraint:owners against table/domain catalogs and synthesizeALTER DOMAIN ... DROP CONSTRAINT/ADD CONSTRAINTfor domain-owned constraints.The coverage bookkeeping now tracks release and restore separately, so existing targeted changes are reused when present while missing halves are added by expansion. This also deduplicates multi-procedure dependencies on the same expression container during the walk.
Sorting was updated so expression restore changes for table/domain CHECK constraints are ordered around procedure replacement, and
AlterDomainDropDefaultis routed through the drop phase so it can release the old function dependency beforeDROP FUNCTIONruns.Regression Coverage
Added RED/GREEN unit coverage for:
NOT VALIDstate for synthesized table/domain CHECK replacements.Added integration coverage in
function-signature-expression-dependencies.test.tsfor PG15/PG17 covering:DROP TABLE;DROP TABLE;DROP TABLE;DROP TABLE;DROP DOMAINor cascadedDROP TABLE;DROP DOMAINorDROP TABLE.Validation
RED evidence was captured in the test commits:
AlterTableAlterColumnSetDefault, missingAlterTableDropConstraint/AlterDomainDropConstraint, and incorrect table CHECK ordering aroundDropProcedure;DROP TABLE/DROP DOMAINoutput for the pg-delta: function replacement can over-recreate tables for default and check-constraint dependencies #280 cases and an apply-time failure for pg-delta: domain CHECK constraint dependents are silently skipped by replacement expansion, producing plans that fail at apply #286.GREEN validation after the implementation:
I also cleaned up the local pg-delta/testcontainers containers after validation.