Repository navigation
fix(migrate-ts): block unowned view column drops and restore SQLite drop-column rollback - #526
Merged
Merged
Conversation
… re-adds the column A live view the tool does not own (@Unmanaged, or outside the scoped schemas) that still reads a dropped column now blocks the drop at plan time, naming the view (--allow drop-column-read-by-view to proceed). The SQLite/D1 down for drop-column re-adds the column instead of writing a TODO. Docs note: edit an @SQL body when removing a column it reads.
… documented in migrations-and-drift.md.
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
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.
What changed
Three separate faults when a column is dropped from under a view. None depends on
@column, which is unchanged, as is the rename guard that refuses a drop-and-add in the same table.--from-db,meta migratenow refuses adrop-columnthat a live view it does not own still reads: a declared@unmanagedview, a hand-made view not in the metadata, or one outside the scoped schemas. The message names the view and the column. Before, the migration was written and failed at apply (Postgres:cannot drop column ... because other objects depend on it; SQLite:error in view ... after drop column). A view with no recorded body is not matched, and offline mode cannot see live views, so it is unchanged.--allow drop-column-read-by-view(allow.dropColumnReadByView, also a config token). Why it exists: the check is a text match on the table and column names, so it can over-report; an operator who has dealt with the view, or knows the match is a false positive, needs a deliberate way past. It follows the existingblockedByand allow-flag pattern used fordrop-view-cascade.drop-columnnow re-adds the column instead of writing a TODO comment, so a rolled-back view works again. Data is not restored, and the migration says so. ANOT NULLcolumn with no default comes back nullable, and a non-constant default is omitted, because SQLite refuses to add such a column to a populated table; each case carries a NOTE line.docs/features/migrations-and-drift.mdand the authoring skill's views reference now say to edit an@sqlbody in the same change that removes a column it reads: the tool never parses it, so Postgres fails at apply and SQLite applies and leaves a broken view. Agent-context fixtures are regenerated.CHANGELOG.md[Unreleased]records all three.Testing
migrate-ts) for both dialects: an unowned view that reads the column is blocked and named, one that does not is not, a body-less view is not, the flag lifts the block, and the SQLite and D1 downs.migrate-ts,codegen-ts,cliandsdksuites pass with 0 failures.--from-dbmode (disposable project): a hand-made view and an@unmanagedview each read the dropped column and the plan is blocked; apply then rollback re-adds the column and the view is queryable.Gate
The no-mistakes gate took 1 run. Review raised the SQLite non-constant-default case (fixed) and asked whether the new allow flag is wanted (kept, per the reason above). Test found that the block did not count a hand-made in-scope view as unowned and fixed it.
The Test step was approved with an exception, not a clean pass: the configured
scripts/ci-local.sh --only ts-fast --only ts-unitwrapper did not finish inside the pipeline's time budget, so its result was not confirmed there, and the live run did not cover Postgres. Hosted checks run the full suite before merge.