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

feat(query-engine): use @map'd values of enum in query input and output #4987

Closed
wants to merge 11 commits into from

Conversation

jkomyno
Copy link
Contributor

@jkomyno jkomyno commented Aug 23, 2024

This PR contributes to prisma/prisma#8446.
This PR introduces a breaking change, let's wait for Prisma 6.

With this PR:

  • The following schema is now invalid:
    datasource db {
      provider  = "postgresql"
      url       = env("TEST_POSTGRES_URI")
    }
    
    enum FormLanguage {
      EN_US     @map("en/us")
      IT_IT     @map("it/it")
      EN_US_DUP @map("en/us")
    
      @@map("form_language")
    }
    
    model Form {
      id        String       @id @default(cuid())
      language  FormLanguage
    }

ba87944 can be extracted into its own PR, as it fixes a prisma validate bug internally discovered here.

@jkomyno jkomyno self-assigned this Aug 23, 2024
@jkomyno jkomyno requested a review from a team as a code owner August 23, 2024 23:40
@jkomyno jkomyno requested review from Weakky and removed request for a team and Weakky August 23, 2024 23:40
Copy link

codspeed-hq bot commented Aug 23, 2024

CodSpeed Performance Report

Merging #4987 will not alter performance

Comparing integration/enum-mapped-values (caa24ef) with main (1886abc)

Summary

✅ 11 untouched benchmarks

Copy link
Contributor

github-actions bot commented Aug 23, 2024

WASM Query Engine file Size

Engine This PR Base branch Diff
Postgres 2.050MiB 2.049MiB 1.034KiB
Postgres (gzip) 822.191KiB 821.958KiB 239.000B
Mysql 2.014MiB 2.013MiB 1.034KiB
Mysql (gzip) 808.071KiB 807.708KiB 372.000B
Sqlite 1.911MiB 1.910MiB 1.034KiB
Sqlite (gzip) 768.207KiB 768.141KiB 67.000B

Copy link
Member

@aqrln aqrln left a comment

Choose a reason for hiding this comment

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

Could you please add some tests where you pass a mapped value in a query (or point me to the existing tests)? I assume GraphQL protocol will probably break for enums with numeric values or special characters like / in your languages example.

@jkomyno
Copy link
Contributor Author

jkomyno commented Nov 28, 2024

Could you please add some tests where you pass a mapped value in a query (or point me to the existing tests)? I assume GraphQL protocol will probably break for enums with numeric values or special characters like / in your languages example.

Right, I've added the new enum_type_mapped test suite in b6ced1e. Locally, I've tested it successfully with Postgres 14.

@jkomyno jkomyno removed this from the 6.0.0 milestone Nov 28, 2024
@jkomyno
Copy link
Contributor Author

jkomyno commented Nov 28, 2024

Updates after some investigation with @aqrln:

  • Currently, this fails:
    model User {
      id       Int        @id @default(autoincrement())
      enumUsed PaymentProvider @map("field_mapping") @default(Offline)
    }
    
    enum PaymentProvider {
      MixplatSMS    @map("mixplat/sms")
      InternalToken @map("internal/token")
      Offline       @map("offline")
    
      @@map("payment_provider")
    }
  • There are possibly other failing cases.
  • Due to this, such breaking change won't be released in Prisma 6.0.0, unfortunately

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants