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.

Sign up for GitHub

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

Jump to bottom

me: fix regression w/ mapped enum names on postgres #3368

Merged
merged 2 commits into from
Nov 9, 2022
Merged

me: fix regression w/ mapped enum names on postgres #3368

merged 2 commits into from
Nov 9, 2022

Commits on Nov 9, 2022

  1. 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.
    @tomhoule
    tomhoule committed Nov 9, 2022
    Configuration menu
    Copy the full SHA
    3389f1a View commit details
    Browse the repository at this point in the history

  2. me: move enum test to enums directory

    @tomhoule
    tomhoule committed Nov 9, 2022
    Configuration menu
    Copy the full SHA
    e607a43 View commit details
    Browse the repository at this point in the history