[Pg-kit]: Drop identity before changing a column's type - #6226
Open
yyuneu wants to merge 2 commits into
Open
Conversation
Postgres allows identity only on integer types, so SET DATA TYPE issued before DROP IDENTITY failed (drizzle-team#4178).
Covers dropping identity while changing the column type and default in one alter.
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.
Addresses the statement ordering reported in #4178. This PR targets
rc5because the issue reproduces with both1.0.0-rc.4and therc5branch.The problem
alterColumnConvertorbuilds statements for a column alteration in a fixed order:type change → default → generated → identity → not-null
PostgreSQL imposes two restrictions on identity columns that conflict with this order:
smallint,integer, orbigint. Even when a cast is available, changing to another type while the identity remains fails withidentity column type must be smallint, integer, or bigint.column "id" of relation "users" is an identity column.When a column drops its identity and changes type in the same alteration, the generated statements run in the wrong order:
SET DATA TYPE …,SET DEFAULT …, thenDROP IDENTITY.For a castable target such as
text(), PostgreSQL rejects the first statement with the identity error above. For the issue'sinteger().generatedAlwaysAsIdentity()→uuid().defaultRandom()example, PostgreSQL reportscannot cast type integer to uuidfirst. The same ordering problem is present, but the missing cast is reported before the identity restriction is checked. The cast limitation is discussed separately below.generateandpushshare this convertor, so both produce the affected statement order. I reproduced the failures withdrizzle-kit@1.0.0-rc.4against PostgreSQL 17.11. The regression test reproduces the identity error on PGlite.Scope
Only identity removal moves earlier. Adding an identity (
from === null) must remain after the type change because PostgreSQL requires an integer type first. Changes to identity parameters do not depend on this ordering. Both cases remain unchanged.alter_columnis the only statement that emits identity changes.alter_column_change_identityis declared instatements.tsbut is never produced.dialects/cockroach/convertor.tsuses the same ordering. I left it unchanged because I do not have a CockroachDB environment to verify the behavior, but I am happy to include it if you would prefer.The fix
When
diff.identity.to === null,DROP IDENTITYis emitted at the start ofalterColumnConvertor, before the type and default blocks.The previous
else ifbranch is kept, with its body replaced by a comment, so the surroundingif/elsechain and its type narrowing remain unchanged.The production change is limited to one file, with six lines added and one removed, and adds one property check at runtime.
Tests
Added one case to
tests/postgres/pg-identity.test.ts, next to the existingdrop identity from a columntests:integer('id').generatedByDefaultAsIdentity()text('id').default('n/a')The test asserts the exact statement order for both
diff(used bygenerate) andpush. Thepushpath also executes the statements on PGlite, so the test fails before the fix with the PostgreSQL identity error, rather than an assertion mismatch.generatedAlwaysAsIdentity()follows the sameDROP IDENTITYpath, so the test exercises the removal logic shared by both variants.All 18 tests in the file pass with the fix.
Verification
rc5withidentity column type must be smallint, integer, or bigintand passes with the fix.kit:postgres,kit:postgres16/17/18,kit:other,kit:cockroach,kit:mssql,prepare(types and lint), andskills-revision-gatepass. The failing shards require Neon, PlanetScale, Turso, or SQLite Cloud credentials.oxlint --max-warnings=0anddprint checkpass.A question for maintainers
The exact
integer → uuidchange from the issue still fails after this fix atSET DATA TYPE uuid USING "id"::uuid, because PostgreSQL has no integer-to-UUID cast. This PR corrects the statement order; the user still needs to supply an appropriate conversion.Would a diagnostic for this type change be useful? I am happy to add one separately so that Kit provides a hint instead of exposing only the PostgreSQL error.
I left
changelogs/drizzle-kit/unchanged because the entries appear to be release-scoped. I am happy to add an entry if needed.