New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prisma 4.6.0 drops and recreates enum field when running db push even if the field has not changed #16180
Comments
Seeing the same behavior here, when we run our diff in CI, we see a number of hits that are not expected, all related to enums:
All of these have @@Map set:
|
Running into the same issue with a similar schema, looks like 4.6.0 might be ignoring the
Resulting migration looks like this:
I was able to reproduce this by:
|
When calculating a SQL schema from a Prisma schema, the Postgres connector of the migration engine would use the name of the enum instead of its database name (see parser-database docs for the reference on what these mean). To picture this with an example, the following schema. ``` enum PersonType { Dog Cat @@Map("pet_preference") } ``` would correspond, before the regression, and after this commit, to: ``` CREATE TYPE "pet_preference" AS ENUM ('Dog', 'Cat'); ``` but the regression caused it to correspond to: ``` CREATE TYPE "PersonType" AS ENUM ('Dog', 'Cat'); ``` This does not only affect newly created enums: we diff the wrong enums, causing bad migrations for existing users of Migrate that have the enums with the correct (mapped) names in their database. Users reported the regression in prisma/prisma#16180 This commit should completely fix the problem. The regression was introduced in an unrelated refactoring in b6400f1. Enum names in column types were replace with enum ids — a win for correctness. As part of this refactoring, the `sql_schema_describer::Enum` type was made private, and adding enums to a schema became possible only through `SqlSchema::push_enum`. In the SQL migration connector What could have caught this: - Testing that the name of the database enum in the migration is the mapped name. We can test this: - Through `.assert_schema()`, which has existed for a long time. It is a gap in our test coverage that we weren't testing this. - The new single migration tests, like the test in this commit. They are new, and they are less work than the previous option. - migrations-ci could have caught this, but it is currently neglected.
Hi there 👋 Quick update from the Prisma folks: we are working on this. There is indeed a regression with enums in 4.6.0, and here's a PR to fix it prisma/prisma-engines#3368 The bug appears when the following two conditions are met:
The regression is that we started using the name of the enum instead of the mapped name (more details in the PR). This is a very embarassing gap in our tests. If you experience a regression with |
@alexisbellido I tried reproducing first using your instructions and could not observe any bad behaviour. Can you try again and confirm if you can really observe undesirable migrations without |
On my team, we have precisely the same behavior |
@hoghweed the behaviour described by @alexisbellido (without |
The one without the |
Ok that's concerning, we can't reproduce what @alexisbellido described, do you have more details that would help us reproduce the issue? edit: thanks a lot for engaging btw |
To make it explicit @hoghweed: We need your help to understand the case where an |
@janpio I know that, unfortunately, as I was saying to Nikolas Burk on Twitter, unfortunately it is very hard to give a context to make this reproducible. Our schema is a simple one with enums, but I'm not sure what is context I can give you. I can just tell you how we found the bug:
This is the only context I can give without giving you the full schema |
Can you run this command please?
(make sure that This should output something like #16180 (comment) Can you then please also share the Prisma schema snippets that define these |
The command result is [*] Changed the `Action` table
[*] Column `result` would be dropped and recreated (type changed)
[*] Changed the `ActionLog` table
[*] Column `kind` would be dropped and recreated (type changed) This is the Prisma snippet.
Consider we don't change those from 5 months, the only change made to the tables was just an indentation change. |
Thanks! We'll try to dig deeper. |
* me: fix regression w/ mapped enum names on postgres When calculating a SQL schema from a Prisma schema, the Postgres connector of the migration engine would use the name of the enum instead of its database name (see parser-database docs for the reference on what these mean). To picture this with an example, the following schema. ``` enum PersonType { Dog Cat @@Map("pet_preference") } ``` would correspond, before the regression, and after this commit, to: ``` CREATE TYPE "pet_preference" AS ENUM ('Dog', 'Cat'); ``` but the regression caused it to correspond to: ``` CREATE TYPE "PersonType" AS ENUM ('Dog', 'Cat'); ``` This does not only affect newly created enums: we diff the wrong enums, causing bad migrations for existing users of Migrate that have the enums with the correct (mapped) names in their database. Users reported the regression in prisma/prisma#16180 This commit should completely fix the problem. The regression was introduced in an unrelated refactoring in b6400f1. Enum names in column types were replace with enum ids — a win for correctness. As part of this refactoring, the `sql_schema_describer::Enum` type was made private, and adding enums to a schema became possible only through `SqlSchema::push_enum`. In the SQL migration connector What could have caught this: - Testing that the name of the database enum in the migration is the mapped name. We can test this: - Through `.assert_schema()`, which has existed for a long time. It is a gap in our test coverage that we weren't testing this. - The new single migration tests, like the test in this commit. They are new, and they are less work than the previous option. - migrations-ci could have caught this, but it is currently neglected. * me: move enum test to enums directory
Thanks indeed @hoghweed. What database are you using (make and version)? Can you maybe share your schema file privately with us at schemas@prisma.io? (Happy to sign NDA if necessary) |
(the few last migrations would also be helpful clues) |
Notes INSERT INTO "public"."User" ("username", "roles") VALUES ('someusername', '{COLLECTOR}'); |
Quick update on this: we think we have a reproduction for the second problem, we're confirming that and working on a PR. |
This regression was reported by users in prisma/prisma#16180 It was introduced on b6400f1 by the same change that caused the _other_ regression that was fixed in eb39236. In that refactoring, we started storing enum ids instead of enum names in `ColumnTypeFamily::Enum(_)`. That makes the `ColumntypeFamily` type smaller, easier to deal with with regards to ownership (`EnumId` is `Copy`), and ready for multi-schema, because the name can be ambiguous and does not carry schema information, whereas the `EnumId` does. But more relevant to this commit: it makes comparing two `ColumnTypeFamily` inaccurate _when the types come from two different schemas_, like in sql_schema_differ. That in turn changed the signature of `column_type_family_as_enum()`, leading to the differ comparing enum ids instead of enum names, which directly causes the false positives in diffing, because the same enum has a different ID in each of the two schemas. The code that caused the regression does not appear in b6400f1, it uses code that was changed there. This is also a problem with the generated implementation of PartialEq for ColumnTypeFamily. Since it would be a larger refactoring, we only deal with this where relevant (sql_schema_differ) and leave removing the dangerous PartialEq implementation to another PR. This is the issue: #3373. How could this have been caught? - Tests that check that migrations are idempotent with multiple enums, all used in model columns _not defined in alphabetical order_ (sad but true). - This is chiming with the theme of us not testing with large enough Prisma Schemas. - migrations-ci could have caught this. We are neglecting it at the moment.
This regression was reported by users in prisma/prisma#16180 It was introduced on b6400f1 by the same change that caused the _other_ regression that was fixed in eb39236. Symptoms: enum migrations became non-idempotent in many cases, depending on the ordering of the migrations in the Prisma schema. In that refactoring, we started storing enum ids instead of enum names in `ColumnTypeFamily::Enum(_)`. That makes the `ColumntypeFamily` type smaller, easier to deal with with regards to ownership (`EnumId` is `Copy`), and ready for multi-schema, because the name can be ambiguous and does not carry schema information, whereas the `EnumId` does. But more relevant to this commit: it makes comparing two `ColumnTypeFamily` inaccurate _when the types come from two different schemas_, like in sql_schema_differ. That in turn changed the signature of `column_type_family_as_enum()`, leading to the differ comparing enum ids instead of enum names, which directly causes the false positives in diffing, because the same enum has a different ID in each of the two schemas. The code that caused the regression does not appear in b6400f1, it uses code that was changed there. This is also a problem with the generated implementation of PartialEq for ColumnTypeFamily. Since it would be a larger refactoring, we only deal with this where relevant (sql_schema_differ) and leave removing the dangerous PartialEq implementation to another PR. This is the issue: #3373. How could this have been caught? - Tests that check that migrations are idempotent with multiple enums, all used in model columns _not defined in alphabetical order_ (sad but true). - This chimes with the theme of us not testing with large enough Prisma schemas. - migrations-ci could have caught this. We are neglecting it at the moment.
Hi, I can reproduce with the steps and schema in my original description and Prisma 4.6.0. As you can see from my basic schema I'm not using @@Map at all. But from what I've read it looks like a few others have experienced the same issue. Thank you for looking into this. |
We will prepare a patch release tomorrow. |
Hi @janpio, We'll provide the details you asked for above. Database: Postgres v12.8 model Account {
id String @id @default(cuid())
name String
type AccountType @default(TRIAL)
sector Sector?
...
}
model SurveyReport {
id String @id @default(cuid())
surveyId String
survey Survey @relation(fields: [surveyId], references: [id], onDelete: SetNull)
reportType SurveyReportType
...
}
enum AccountType {
ORGANISATION
TRIAL
...
}
enum Sector {
PRIVATE_SECTOR
PUBLIC_SECTOR
...
}
enum SurveyReportType {
SPAM
OTHER
...
} We were creating a new migration based off the latest Prisma validation error: Here is a screenshot of the diff between the changes we tried to make: This ultimately resulted in a migration file, which attempted to drop rows within other models that referenced enums. Interestingly, the migration file created on v4.6.0 attempts to drop rows that reference the enum |
* me: fix regression w/ mapped enum names on postgres When calculating a SQL schema from a Prisma schema, the Postgres connector of the migration engine would use the name of the enum instead of its database name (see parser-database docs for the reference on what these mean). To picture this with an example, the following schema. ``` enum PersonType { Dog Cat @@Map("pet_preference") } ``` would correspond, before the regression, and after this commit, to: ``` CREATE TYPE "pet_preference" AS ENUM ('Dog', 'Cat'); ``` but the regression caused it to correspond to: ``` CREATE TYPE "PersonType" AS ENUM ('Dog', 'Cat'); ``` This does not only affect newly created enums: we diff the wrong enums, causing bad migrations for existing users of Migrate that have the enums with the correct (mapped) names in their database. Users reported the regression in prisma/prisma#16180 This commit should completely fix the problem. The regression was introduced in an unrelated refactoring in b6400f1. Enum names in column types were replace with enum ids — a win for correctness. As part of this refactoring, the `sql_schema_describer::Enum` type was made private, and adding enums to a schema became possible only through `SqlSchema::push_enum`. In the SQL migration connector What could have caught this: - Testing that the name of the database enum in the migration is the mapped name. We can test this: - Through `.assert_schema()`, which has existed for a long time. It is a gap in our test coverage that we weren't testing this. - The new single migration tests, like the test in this commit. They are new, and they are less work than the previous option. - migrations-ci could have caught this, but it is currently neglected. * me: move enum test to enums directory
This regression was reported by users in prisma/prisma#16180 It was introduced on b6400f1 by the same change that caused the _other_ regression that was fixed in eb39236. Symptoms: enum migrations became non-idempotent in many cases, depending on the ordering of the migrations in the Prisma schema. In that refactoring, we started storing enum ids instead of enum names in `ColumnTypeFamily::Enum(_)`. That makes the `ColumntypeFamily` type smaller, easier to deal with with regards to ownership (`EnumId` is `Copy`), and ready for multi-schema, because the name can be ambiguous and does not carry schema information, whereas the `EnumId` does. But more relevant to this commit: it makes comparing two `ColumnTypeFamily` inaccurate _when the types come from two different schemas_, like in sql_schema_differ. That in turn changed the signature of `column_type_family_as_enum()`, leading to the differ comparing enum ids instead of enum names, which directly causes the false positives in diffing, because the same enum has a different ID in each of the two schemas. The code that caused the regression does not appear in b6400f1, it uses code that was changed there. This is also a problem with the generated implementation of PartialEq for ColumnTypeFamily. Since it would be a larger refactoring, we only deal with this where relevant (sql_schema_differ) and leave removing the dangerous PartialEq implementation to another PR. This is the issue: #3373. How could this have been caught? - Tests that check that migrations are idempotent with multiple enums, all used in model columns _not defined in alphabetical order_ (sad but true). - This chimes with the theme of us not testing with large enough Prisma schemas. - migrations-ci could have caught this. We are neglecting it at the moment.
Hey, @tomhoule, I couldn't replicate the original issue with the sample schema I provided (without Here's the basic schema to try. model User {
id Int @id @default(autoincrement())
username String @unique @db.VarChar(32)
roles Role[] @default([COLLECTOR])
nfts NFT[]
}
enum Role {
ADMIN
COLLECTOR
}
model NFT {
id Int @id @default(autoincrement())
owner User @relation(fields: [ownerId], references: [id])
ownerId Int @unique
images NFTImage[]
}
model NFTImage {
id Int @id @default(autoincrement())
nft NFT @relation(fields: [nftId], references: [id])
nftId Int
title String? @db.VarChar(64)
src String? @db.VarChar(128) // URL pointing to image
alt String? @db.VarChar(64)
orientation ImageOrientation @default(VERTICAL)
}
enum ImageOrientation {
HORIZONTAL
VERTICAL
} Then reset with this.
and insert one row to the INSERT INTO "User" (username, roles) VALUES ('someusername', '{COLLECTOR, ADMIN}'); now, and without changing anything else, run Here's my output from
I think I started seeing the problem again when I added the relations between |
Thanks @alexisbellido
and for -- AlterTable
ALTER TABLE "User" DROP COLUMN "roles",
ADD COLUMN "roles" "Role"[] DEFAULT ARRAY['COLLECTOR']::"Role"[];
-- AlterTable
ALTER TABLE "NFTImage" DROP COLUMN "orientation",
ADD COLUMN "orientation" "ImageOrientation" NOT NULL DEFAULT 'VERTICAL'; This does not happen on |
Hi @alexisbellido — I confirmed in a test with your example schema that prisma/prisma-engines@b4992e9 fixes the issue. Stay tuned for the patch release. |
@jameyhart It seems to me you are experiencing two things here at the same time: We changed our validations around |
Came here looking for this because I just bumped to 4.6 to get Postgres native upserts (yay!) but now I'm seeing the same issue with Prisma attempting to drop and recreate all enum fields after trying to add a new migration to our schema. Glad it's already on the radar with a fix pending. |
Is this comment still applicable, I do not have |
Thanks @alexisbellido. Fingers crossed it's the same underlying issue and 4.7 will resolve the issue for us also. Thankfully I caught this before any of the team made any schema changes. |
@alexisbellido @rcbevans @bsproul @hoghweed @davidlumley We will wait for some confirmations before we publish the official patch to latest. |
@Jolg42 |
@Jolg42 , I just tested Thank you so much, Prisma team. |
The official patch is out in |
Update Prisma from `4.6.0` to `4.6.1` to solve enum issue with PostgreSQL prisma/prisma#16180
Update Prisma from `4.6.0` to `4.6.1` to solve enum issue with PostgreSQL prisma/prisma#16180
Bug description
I just updated from Prisma 4.5.0 to 4.6.0 and now every time I run
npx prisma db push
, even if I haven't changed anything inschema.prisma
, Prisma wants to drop and recreate an enum field. I get this message:If I choose to ignore the warning all my data in the
roles
column is reset.After testing with multiple changes to my schema and simplifying it to the most basic form (included below) I downgraded to Prisma 4.5.0 and everything works as expected there. I've reconfirmed the problem exists by going back to Prisma 4.6.0.
How to reproduce
schema.prisma.
Pay attention to theroles
field, which is an enum.roles
field.npx prisma db push
multiple times without changing anything inschema.prisma
.The
rolescolumn on the
Usertable would be dropped and recreated. This will lead to data loss
, and if you accept the warning the data in theroles
column will be reset.Expected behavior
The
roles
enum column is not being changed inschema.prisma
so it shouldn't be dropped and recreated when runningnpx prisma db push
.Prisma information
// Add your code using Prisma Client
Environment & setup
Prisma Version
The text was updated successfully, but these errors were encountered: