Skip to content
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

Fix a crash with a special case of dbgenerated() #3515

Merged
merged 2 commits into from Dec 15, 2022

Conversation

pimeys
Copy link
Contributor

@pimeys pimeys commented Dec 15, 2022

This bug is triggered by the following combination of things:

  • MySQL
  • Use of dbgenerated() (a deliberatly empty one)
  • Native type of Time

If you have this in the PSL, pushing twice will panic.

What happens is two things: we think the value time(0) from the database is diffed as a different value what is Time in the PSL. This is unneeded drift and should not happen.

The second issue is when the change is adding a dbgenerated() to a field, which is not really done by the users due to it not being very useful (and a legacy structure). Now, our MySQL rendering does not think somebody would add a dbgenerated() to the PSL. But, if you combine it with thinking db.Time and time(0) are different, getting another migration from the second push... Now we hit the unreachable() and crash.

Closes: prisma/prisma#16340
Closes: prisma/prisma#9769
Closes: prisma/prisma#6382

This bug is triggered by the following combination of _things_:

- MySQL
- Use of `dbgenerated()` (a deliberatly empty one)
- Native type of `Time`

If you have this in the PSL, pushing twice will panic.

What happens is two things: we think the value `time(0)` from the
database is diffed as a different value what is `Time` in the PSL.
This is unneeded drift and should not happen.

The second issue is when the change is adding a `dbgenerated()` to a
field, which is not really done by the users due to it not being very
useful (and a legacy structure). Now, our MySQL rendering does not
think somebody would _add_ a `dbgenerated()` to the PSL. But, if you
combine it with thinking `db.Time` and `time(0)` are different,
getting another migration from the second push... Now we hit the
`unreachable()` and crash.
@pimeys pimeys added this to the 4.8.0 milestone Dec 15, 2022
@pimeys pimeys requested a review from a team as a code owner December 15, 2022 15:02
Copy link
Contributor

@tomhoule tomhoule left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great! three bugs down

@pimeys pimeys merged commit d4c8ebe into main Dec 15, 2022
@pimeys pimeys deleted the migration-engine/fix-crash-with-empty-dbgenerated branch December 15, 2022 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants