prisma / prisma-engines

🚂 Engine components of Prisma ORM
https://www.prisma.io/docs/concepts/components/prisma-engines
Apache License 2.0
1.2k stars 240 forks source link

Miss matching type in datamodel #2586

Open hyochan opened 2 years ago

hyochan commented 2 years ago

I think there is miss matching data type in datamodel in the Prisma engine.

If I get DMMF from below DML,

model User {
  id    Int @id
  email String
  name  String?
}

After running the below code,

getDMMF({
    datamodel: samplePrismaSchema,
}).datamodel.models

The result is below.

datamodel [
  {
    name: 'User',
    dbName: null,
    fields: [
      {
        name: 'id',
        kind: 'scalar',
        isList: false,
        isRequired: true,
        isUnique: false,
        isId: true,
        isReadOnly: false,
        type: 'Int',
        hasDefaultValue: true,
        default: { name: 'autoincrement', args: [] },
        isGenerated: false,
        isUpdatedAt: false
      },
      {
        name: 'email',
        kind: 'scalar',
        isList: false,
        isRequired: true,
        isUnique: true,
        isId: false,
        isReadOnly: false,
        type: 'String',
        hasDefaultValue: false,
        isGenerated: false,
        isUpdatedAt: false
      },
      {
        name: 'name',
        kind: 'scalar',
        isList: false,
        isRequired: false,
        isUnique: false,
        isId: false,
        isReadOnly: false,
        type: 'String',
        hasDefaultValue: false,
        isGenerated: false,
        isUpdatedAt: false
      }
    ],
    isGenerated: false,
    primaryKey: null,
    uniqueFields: [],
    uniqueIndexes: []
  }
]

Shouldn't primaryKey be id? Also the uniqueFields didn't show correctly when I added them.

If what I am thinking is a correct output, I wanted to contribute to the fix myself but I am not sure how to run the datamodel_converter_tests.rs test in my machine. Hope someone could guide me on this if possible too.

I think modifying the below code would fix some how.

https://github.com/prisma/prisma-engines/blob/6761f920881589dbeade9c5ad55fcab533740ddf/query-engine/prisma-models/src/builders/internal_dm_builder.rs?_pjax=%23js-repo-pjax-container%3Afirst-of-type%2C%20div%5Bitemtype%3D%22http%3A%2F%2Fschema.org%2FSoftwareSourceCode%22%5D%20main%3Afirst-of-type%2C%20%5Bdata-pjax-container%5D%3Afirst-of-type&_pjax=%23js-repo-pjax-container#L234-L239

tomhoule commented 2 years ago

Hi @hyochan ! I think the id on model is false because it's an @id on a field rather than an @@id directly on the model. Tagging the client team for a better answer.

hyochan commented 2 years ago

@tomhoule Thanks Tom for the reply! I think the @id should be the primary key based on the prisma doc.

Correct me if I am misunderstanding something.

Screen Shot 2022-01-16 at 4 54 49 PM

Also, I've tested uniqueFields with @unique and it's always [].

janpio commented 2 years ago

What @tomhoule was trying to say still holds true:# On the model level you are looking at, where you have primaryKey = false and uniqueFields = [], there is no primary key defined with @@id and no uniques with @@unique. What you do have is a primary key via @id on a specific field, that has isId: true, in the data. Similar a @unique should make a field isUnique: false,.

hyochan commented 2 years ago

@janpio Thanks for the detail. I am a bit confused though. How about the below case?

model User {
  id    Int
  email String
  name  String?

  @@id(id)
  @@unique([id, email])
}

I see the result below.

datamodel [
  {
    name: 'User',
    dbName: null,
    fields: [ [Object], [Object], [Object] ],
    isGenerated: false,
    primaryKey: null,
    uniqueFields: [],
    uniqueIndexes: []
  }
]
janpio commented 2 years ago

That would be a valid bug report then or an indication that I misunderstood how this works. (@tomhoule?)

tomhoule commented 2 years ago

Yes, looks like a bug.

janpio commented 2 years ago

Considering the Engine and Client is still doing the right thing, this might just be not used the way we expect it to. Dmmf is an internal format after all, so we might very well be using things in a way that from the outside looks like a "bug".

Client team will need to look into this.