Repository navigation
Fix postgres and CockroachDB diff cases that write failing SQL around moves, renames, roles and generated columns - #6511
Open
dankochetov wants to merge 2 commits into
Open
dankochetov wants to merge 2 commits into
dankochetov wants to merge 2 commits into
Conversation
… moves, renames, roles and generated columns Postgres: - Count the brackets of a custom type's SQL type as array dimensions. - Drop and create the foreign keys of a recreated generated column one time each, and create its indexes and unique constraints one time. - Rename a view in its old schema before it moves, and move it with its old kind. - Drop a recreated view after its rename or move. - Run the policy drops, foreign key drops and row level security statements of a moved table after `SET SCHEMA`. - Run `DROP ROLE` after the policy statements. - Run `DROP EXPRESSION` first and write no `USING` clause for a generated column. - Find the enum columns that a format 7 snapshot stores without a type schema when the enum is recreated, with the s06 tests and fixtures. CockroachDB: - Drop a view that reads a renamed or moved table before the table statement and create it after it. Replace `recreate_view` with a `drop_view` that has a `cause`, in the diff, in push and in `cockroachExplain`. - Write both statements for a view or a sequence that moves and gets a new name, and look up the old view of a moved view that is recreated. - Name the table in an index rename or drop when two tables hold the index name. - Write `RENAME CONSTRAINT` for a check rename. - Select the row of the renamed check, index, primary key, foreign key or policy by its table. - Change the column type when only the type schema or the array dimensions change, and count the brackets of a custom type's SQL type. - Run table renames and moves before the statements that name the table in its new place. - Create the foreign keys of a recreated generated column again. - Run `DROP ROLE` after the policy statements.
dankochetov
force-pushed
the
postgres-cockroach-diff-fixes
branch
from
October 15, 2026 14:49
b71eb89 to
0d3183f
Compare
dankochetov
marked this pull request as ready for review
October 15, 2026 15:06
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.
This PR targets the
snapshotsbranch. Its base moves torc5after #6503 merges.What it adds
The tests of #6503 found postgres and CockroachDB diff cases where
generateorpushwrites SQL that fails, loses a change, or never converges. rc5 has the same bugs. This PR fixes them.Postgres
customType({ dataType: () => 'text[]' })without.array(), is an array column.fromDrizzleSchemacounts the[]and[n]brackets of the SQL type fordimensionsand removes them fromtype.ALTER VIEW.generatewithMissing view in original ddl.SET SCHEMA, because they name the table in its new schema. Table drops still run before the moves.DROP ROLEruns after the policy statements, because Postgres refuses to drop a role that a policy still names.DROP EXPRESSIONruns before the type and default statements of the same column, and a generated column gets noUSINGclause. Postgres refuses a default and aUSINGclause for a generated column.intervalcolumn is not taken for a column of an enum namedinterval.CockroachDB
recreate_viewis now adrop_viewwithcauseand acreate_view, as in postgres.pushasks no data loss question for such a view, and--explainshows it one time.generatewithMissing view in original ddl."table"@"name") when two tables of the schema hold the index name at that point of the migration.RENAME CONSTRAINT. A check that takes the name of a dropped unique constraint is renamed after theDROP INDEX.SET DATA TYPEruns when only the type schema or the array dimensions of a column change, and an enum-to-enum cast across schemas goes throughtext. A custom type that names an enum equals a column of that enum, and a custom type whose SQL type names an array type is an array column.DROP ROLEruns after the policy statements.Why it is a separate PR
These are fixes for rc5 bugs. Format 9 and the check of branches in #6503 do not need them. A separate PR keeps the review of #6503 smaller, and each fix here can be reviewed against rc5 behavior.
Tests
Postgres (PGlite):
pg-array.test.ts: 'a custom type with the type text $dataType is an array column' (text[]andtext[3]).pg-generated.test.ts: thirteen tests named 'a new expression of a generated column ...', 'a new expression of two generated columns ...' and 'a column that becomes generated with a renamed foreign key ...' cover the foreign keys, indexes and unique constraints of a recreated column. '$name gets a type change that runs' coversDROP EXPRESSIONand the missingUSING.pg-views.test.ts: 'a view that moves to another schema with a new name and a new definition writes SQL that runs', 'a view that moves to another schema and becomes a materialized view writes SQL that runs', 'move view to a new schema and change its definition' and 'rename view and change its definition'.pg-tables.test.ts: 'a table move together with $name writes SQL that runs' (policy drops, the first policy, the RLS flag, a foreign key drop) and 'a table move into a schema where a dropped table holds the name of its primary key writes SQL that runs'.pg-role.test.ts: 'a role drop together with $name writes SQL that runs'. The test 'create and drop role' now expectsCREATE ROLEbeforeDROP ROLE.pg-snapshot-v7.test.tswith the fixturessnapshots/schema06.tsandsnapshots/schema06new.ts: the three s06 tests for the first diff after the upgrade from format 7.CockroachDB:
views.test.ts: '$name is dropped before the table statement and created after it', 'a view that is created again keeps its place when it does not read the table that moves', the twoexplaintests, 'push asks for no confirmation when it creates a materialized view with rows again', 'a table rename leaves a view alone that reads a table of a schema with the new table name', and the view move tests with a new name, a new definition or a new kind.indexes.test.tsandconstraints.test.ts: index renames that swap a name, an index drop and a rename onto a name that another rename frees, a unique index whose name a new table takes, an index rename next to a primary key with the same name, and the swap tests of foreign keys and primary keys.checks.test.ts: the check rename statement, a swap of check names between two tables, and a rename to the name of a dropped unique constraint.policy.test.ts: 'a policy rename on one table keeps the change of a policy with the same name on another table'.enums.test.tsandarray.test.ts: the enum switches across and inside schemas, a custom type that names an enum, scalar and array type changes, and a custom type that names an array type.tables.test.ts: 'a table move together with $name writes SQL that runs', the move with a dropped referenced table, and the rename with the RLS flag.generated.test.ts: 'a new expression of a generated column $name'.role.test.ts: 'a role drop together with $name writes SQL that runs'. 'create and drop role' and 'rename role - recreate' now expectCREATE ROLEbeforeDROP ROLE.sequences.test.ts: 'a sequence that moves to another schema with a new name and new fields writes SQL that runs'.