fix(pg-delta): order dependent view drops before column type rewrites#278
Closed
avallete wants to merge 5 commits into
Closed
fix(pg-delta): order dependent view drops before column type rewrites#278avallete wants to merge 5 commits into
avallete wants to merge 5 commits into
Conversation
…column rewrite
Adds a sorter unit test and an end-to-end roundtrip integration test for the
case where an ALTER COLUMN ... TYPE rewrite is referenced by a view that is
dropped and recreated in the same plan.
RED against current code (drop phase has no producer for the rewritten
column, so the main-catalog pg_depend edge can't order the dependent drop):
expect(received).toEqual(expected)
- "DropView"
"AlterTableAlterColumnType"
+ "DropView"
"CreateView"
i.e. the rewrite is emitted before the dependent DROP VIEW, which PostgreSQL
rejects with 0A000 "cannot alter type of a column used by a view or rule".
https://claude.ai/code/session_01Fnhs5Jt3LhiLQzF1c8JguH
An ALTER COLUMN ... TYPE rewrite fails when the original column is still referenced by a view that is dropped and recreated in the same plan. The diff already produced the right change set (AlterTableAlterColumnType, DropView, CreateView); only the order was wrong. getExecutionPhase already routes a built-in-target rewrite into the drop phase, but the rewrite only had a `requires` edge for the column, so the drop phase had no producer for that column stable id. Without a producer, the main-catalog pg_depend edge from the view to the column could not order DROP VIEW before the rewrite. Rather than couple the generic graph builder to a specific change subtype, introduce a first-class `invalidates` member on Change (sibling to creates/drops/requires). It declares the stable ids a change mutates in place: the object keeps its identity, but dependents bound to the old definition must be torn down before the change and rebuilt after. AlterTableAlterColumnType reports its column; buildGraphData folds `invalidates` into the drop-phase producer set exactly like `drops`, so the existing pg_depend edges order dependents' drops ahead of the rewrite. This is ordering-only: `invalidates` does not feed Change.drops, so phase assignment, filtering, fingerprints, and serialization are unchanged. Recreation order needs no help because the create phase runs entirely after the drop phase. GREEN: - src/core/sort/sort-changes.test.ts: 3 pass - tests/integration/alter-table-operations.test.ts (pg17): 22 pass (focused "change column type after dropping dependent view" snapshot emits DROP VIEW before the ALTER COLUMN ... TYPE) https://claude.ai/code/session_01Fnhs5Jt3LhiLQzF1c8JguH
🦋 Changeset detectedLatest commit: 86d80b1 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 |
commit: |
avallete
changed the title
Add a bullet to the Cycle Breaking / Normalization taxonomy describing when to declare `invalidates` (in-place mutations that invalidate dependents) so the next contributor reaches for the Change member instead of an instanceof in the generic graph builder. https://claude.ai/code/session_01Fnhs5Jt3LhiLQzF1c8JguH
The integration test snapshotted the full recreated CREATE VIEW body, but pg_get_viewdef output (column qualification, whitespace) varies across PostgreSQL builds — CI deparsed `alter_column_type_view_dependent_users.age` while the locally-pulled image emitted bare `age`, so the inline snapshot failed on PG15/PG17 shards even though the fix ordered the statements correctly. Assert the deparse-independent property the fix actually guarantees instead: DROP VIEW before the in-place ALTER COLUMN ... TYPE, and the rewrite before the recreated CREATE VIEW. roundtripFidelityTest still applies the SQL, so a wrong order would also fail at apply with 0A000. The exact ordering remains pinned deterministically by the sort-changes unit test. https://claude.ai/code/session_01Fnhs5Jt3LhiLQzF1c8JguH
Member
Author
|
Closing in favor of: #273 |
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
2 participants
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.
Summary
Fixes ordering of schema changes when a column type is altered in place and dependent views exist. Views must be dropped before the column type rewrite and recreated after, since the rewrite invalidates the view's definition.
Changes
Added
invalidatesproperty toBaseChange: A new getter that returns stable identifiers of objects that are mutated in place. Unlikedrops, the object keeps its identity but its dependents must be torn down and rebuilt around the mutation.Implemented
invalidatesinAlterTableAlterColumnType: Reports the column being altered as invalidated, sinceALTER COLUMN ... TYPErewrites the column in place and breaks dependent views.Updated graph builder to handle invalidations: In the drop phase, invalidated ids are treated like dropped ids for ordering purposes — dependents are ordered before the mutation, ensuring views are dropped before the column type change.
Added test coverage:
sort-changes.test.tsverifying correct ordering:DropView→AlterTableAlterColumnType→CreateViewalter-table-operations.test.tsvalidating the full roundtrip with actual database operationsAdded changeset for patch release
Implementation Details
The
invalidatesmechanism is distinct fromdrops:drops: Object is removed entirely; phase assignment, filtering, and serialization all changeinvalidates: Object is mutated in place; only ordering is affected via the dependency graphThis allows the sorter to use
pg_dependedges to order dependent teardowns before the mutation, without changing how the mutation itself is classified or serialized.https://claude.ai/code/session_01Fnhs5Jt3LhiLQzF1c8JguH