prisma / specs

โš  This repository is not actively used any more, please check out the Prisma Documentation for updated information on Prisma. โš 
https://docs.prisma.io
87 stars 16 forks source link

Id Strategies + Sequences #176

Closed mavilein closed 4 years ago

mavilein commented 5 years ago

Currently we map all fields of type Int @id always to an int column that is backed by a sequence. Currently it is not possible to have an int column as primary key that is not backed by a sequence.

Example: The following example currently always maps to an int id field that is backed by a sequence.

model User {
  id Int @id
}

The spec for PSL does not say anything about the desired behavior in this case at all. We need this to start work on the related epic.

matthewmueller commented 5 years ago

matt's note: @id jumps to definition postgres.prisma

matthewmueller commented 5 years ago
Screen Shot 2019-10-10 at 11 24 53 AM Screen Shot 2019-10-10 at 11 23 30 AM Screen Shot 2019-10-10 at 11 25 02 AM
matthewmueller commented 5 years ago

I'm in favor of @mavilein's proposal where @id

implies:

does not imply:


One consideration according to @do4gr: SQLite allows for null primary keys. I don't think this is a use-case we want to support, but if there are existing SQLite databases in the wild with null primary keys, we may run into problems.

mavilein commented 5 years ago

@matthewmueller : Have we ever thought of @id @default(autoIncrement()) instead of @id @pg.serial. Would be kind of nice if all the auto generated id stuff would be handled through @default(...).

matthewmueller commented 5 years ago

Interesting idea! Two questions:

1. What would happen if I pass it a NULL value during an update?

photon.users.find({ id: 10 }).update({ id: null })

Would it maintain the same autoincremented value or would it take the next value?

2. Does it work for other types?

model User {
  createdAt DateTime default(autoincrement())
}
mavilein commented 5 years ago

@matthewmueller :

  1. What would happen if I pass it a NULL value during an update?

Hmm not sure. We would need to define what @default exactly does. My understanding so far was that it just kicks in on creates where the value is not provided by the user.

  1. Does it work for other types?

No only for Ints i guess. I would not know what to expect from incrementing a DateTime. One more millisecond, second, hour, day, year? ๐Ÿคทโ€โ™‚

mavilein commented 5 years ago

A different way to look at this problem is opt out vs. opt in: Most of the time users want auto generated ids anyway. This is the reasoning for the current behavior. We just need a way for users to opt out of auto generated ids. Our old spec rather biases to opt in which seems weird in that case.

matthewmueller commented 5 years ago

A different way to look at this problem is opt out vs. opt in: Most of the time users want auto generated ids anyway. This is the reasoning for the current behavior. We just need a way for users to opt out of auto generated ids.

This makes sense to me. And I think we can do this in the same way as we export standard types that most people use String, Int, Float. We'd export @id = @autoincrement @key

matthewmueller commented 5 years ago

Problem

Right now it's unclear what @id implies.

Having @id imply auto-incremented doesn't work for a lot of existing databases.

Goals

  1. Come up with a common schema syntax that we can put in our docs that's common across most of the databases and is easy to remember. I believe we have achieved this already with our current syntax.

  2. Make sure it doesn't hold us back from weird database setups in the wild. This is what we don't have yet with our current implementation and is something we'd like to add without degrading our first goal.

Potential Solution

Mental model: Follow what the databases do here as closely as possible. This spec builds on the postgres.prisma file that we introduced in #291.

Postgres #### postgres.prisma ```groovy attr id @raw("primary key") type Sequence @raw("serial") ``` #### schema.prisma If you want an integer primary key that autoincrements: ```groovy model User { id Sequence @id } ``` If you want a integer that doesn't autoincrement: ```groovy model User { id Int @id } ``` If you want a string primary key: ```groovy model User { id String @id } ``` If you want a string primary key that has a default: ```groovy model User { id String @id @default(uuid()) } ```
MySQL #### mysql.prisma ```groovy attr id @raw("primary key") attr autoincrement @raw("auto_increment") type Integer @raw("integer") type Sequence = Integer @autoincrement ``` #### schema.prisma If you want an integer primary key that autoincrements: ```groovy model User { id Sequence @id } ``` If you want a integer that doesn't autoincrement: ```groovy model User { id Int @id } ``` If you want a string primary key: ```groovy model User { id String @id } ``` If you want a string primary key that has a default: ```groovy model User { id String @id @default(uuid()) } ```
SQLite #### sqlite.prisma ```groovy attr id @raw("primary key") attr autoincrement @raw("autoincrement") type Integer @raw("integer") type Sequence = Integer @autoincrement ``` #### schema.prisma If you want a integer primary key that autoincrements: ```groovy model User { id Sequence @id } ``` If you want a integer that doesn't autoincrement: ```groovy model User { id Int @id } ``` If you want a string primary key: ```groovy model User { id String @id } ``` If you want an string primary key that has a default: ```groovy model User { id String @id @default(uuid()) } ```

Open Questions

1. What do to about optionals with `@id`? Most databases don't allow this: ```groovy model A { id Int? @id } ``` Personally, I think this should be a lint & compile error, but how can we make this clear in the datasource schemas (e.g. `postgres.schema`)?
2. Can we improve the final syntax? Postgres makes this one difficult because sequences are a data type. I didn't plan to have `Serial @id`, but that's the only syntax I could come up with that would work consistently across the board. I also considered using this syntax: ```sql CREATE SEQUENCE table_name_id_seq; CREATE TABLE table_name ( id integer NOT NULL DEFAULT nextval('table_name_id_seq') ); ``` But couldn't think of a decent way to make the `nextval()` work.

Background Research #### With Primary Key ```sql create table a ( id integer primary key ); ``` - `primary key` in **Postgres** implies not null and unique - `primary key` in **SQLite** implies not null, unique, and auto-increment - SQLite will still work with a null id, it just ignores the null and auto-increments - SQLite will look like it works inserting a pre-existing id, but it will silently ignore the insert - `primary key` in **MySQL** implies not null and unique - `primary key` in **MongoDB** todo #### Without Primary Key ```sql create table a ( id integer ); ``` - Allowed in **Postgres** default is null, no unique, no auto-increment - Allowed in **SQLite** default is null, no unique, no auto-increment - Allowed in **MySQL** default is null, no unique, no auto-increment - **MongoDB** todo
matthewmueller commented 5 years ago

@mavilein's feedback (reposted from DM):

Have you thought about something like this maybe?

Auto Generated Ids:
Int @id
String @id(cuid())
Stirng @id(uuid())

Non Auto Genrated Ids:
Int @id(MANUAL)
String @id(MANUAL)

would error:
String @id 
matthewmueller commented 5 years ago

@mavilein

Serial is not a well known name outside the Postgres community.

Agreed. I was hard-pressed to come up with a more concise name though. Autoincrement seemed way too long. Maybe Sequence?

The definition for the Serial alias in SQLite and MySQL looks wrong as it already contains @id. Hence @id would not be needed in the model definitions anymore.

Good catch! They should just be Serial = Integer @autoincrement. I'll update.

The proposal is not showing an example for an integer id that is not auto incrementing. This is a common usecase when you integrate with legacy systems. Adding this example will probably look weird next to the corresponding examples for string ids that are auto / non auto generated.

Ah yah, I started feeling like my examples were getting too long, I've thought about this though. I've added this and turned the examples into dropdowns so hopefully it's more concise.

I think we need to make auto generated ids the default and it must be more work to opt out. Originally we already allowed id String @id. But the problem was that users were not aware that this resulted in an id field that does not have an auto generated value. Thatโ€™s why i decided as a temporary fix to reject this in the parser and force people via the error message to use e.g. @id @default(cuid()). I think this must be the core consideration for the id field.

I think I would also be fine with:

model User {
  id Int @id // Implies non-null, unique, auto-incrementing
}

model Post {
  id String @id // Error: must specify a default, for example: @default(uuid())
}

Not as huge of a fan of:

model User {
  id String @id(cuid())
}

because you can end up in weird situations like:

model User {
  id String @id(cuid()) @default(uuid())
  // does @id beat @default?
}

or

model User {
  id String @id @default(uuid())
  // is this valid?
}

This design makes the assumption that we have more than just our 4-5 core data types and we also have a Sequence-like data type. This is inline with how Postgres works, but I can see that being... unfamiliar right now.

mavilein commented 5 years ago

If we disallow id String @id we must provide a different way for defining a String id that does not get auto generated.

Regarding the weird situation with @id and @default: We would simply disallow @default for this case.

matthewmueller commented 4 years ago

If we disallow id String @id we must provide a different way for defining a String id that does not get auto generated.

This would be resolved by the proposal. In your schema.prisma:

If you want an integer primary key that autoincrements:

model User {
  id Sequence @id
}

If you want a integer that doesn't autoincrement:

model User {
  id Int @id
}

If you want a string primary key:

model User {
  id String @id
}

If you want a string primary key string that has a default:

model User {
  id String @id @default(uuid())
}

Do you have any issues with treating sequences as another data type?

Regarding the weird situation with @id and @default: We would simply disallow @default for this case.

Good point, that's another option. I'm still leaning towards having sequences be another data type (how it works in postgres) unless we discover a reason not to.

matthewmueller commented 4 years ago

To map out the proposals:

Proposal A

Use Case Syntax (on single line due to formatting constraints)
primary key integer with autoincrement model User { id Sequence @id }
primary key integer without autoincrement model User { id Int @id }
primary key string with default model User { id String @id @default(uuid()) }
primary key string without default model User { id String @id }

Pros

Cons


Proposal B

Use Case Syntax (on single line due to formatting constraints)
Integer primary key with autoincrement model User { id Int @id }
Integer primary key without autoincrement model User { id Int @id(MANUAL) }
string primary key with default model User { id String @id(cuid()) }
string primary key without default model User { id String @id(MANUAL) }

Pros

Cons


I'm not sure there's a great solution either way, but I'm curious to hear what other folks think.

mavilein commented 4 years ago

Here's some more explanation of the first con of the proposal of @matthewmueller:

Leading up to to Prisma Day we already allowed the syntax model User { id String @id } to define an id field of type String where the value is not auto generated. During that time i have gotten a lot of bug reports because users were using that notation and were expecting the id to be auto generated. Our users were not understanding the semantics. Apparently this approach looks nice but is not intuitive. Even our team members did not understand it. To avoid this confusion i decided to disallow this notation and provide a nice error message asking the user to use either model User { id String @id @default(uuid()) } or model User { id String @id @default(cuid()) } instead.

So my concern is that we would run into the same problem again. Therefore the motivation for my proposal was to design the syntax in a way that id fields without auto generation are opt out. And therefore it is less likely that users fall into this trap.

idan commented 4 years ago

My $0.02: I generally don't believe that most people will be writing PSL themselves but introspecting it. The ones who are writing it manually, I want to be nice to, but I am also fine requiring that they wrap their heads around some syntax.

If model User {id String @id @default(whatever)} is the clearest way we have to express something, then that's what we got. While I would prefer a terse syntax for this most common use-case, I wouldn't get too stuck on it now. Also, not sure how this would interact with the whole custom/schema types debate that is ongoing.

@mavilein @matthewmueller what's the actionable thing that needs to happen here? What's the minimum amount of decisionmaking that unblocks engineering?

idan commented 4 years ago

btw, I'm okay with requiring the @default(whatever) when the type is a String.

"You must choose a sequence method when the column type is String" or similar error message.

What's the opt-out syntax look like? model User { id String @id @??? }

matthewmueller commented 4 years ago

I think the personas will resolve a lot of these kinds of discussions. If we're focusing on the introspection case for MVPhoton, then forgetting to add @default() is no longer something we need to care about right now.

model User {id String @id @default(whatever) } is also inline with how SQL works:

create extension "pgcrypto";

create table User (
  id uuid primary key default gen_random_uuid()
);

The 2nd part of this discussion is around introducing a datatype call Sequence. Proposal A has the unfortunate affect of being both a datatype (integer) and a behavior (auto-increment). Proposal B has uglier opt-out. Proposal A is based on what Postgres does:

create table User (
  id serial primary key
);

or

create sequence autoincrement;

create table User (
  id autoincrement primary key
);

Important Note: This decision only affects primary keys that are integers.


Another idea where you can attach behaviors to data types, sort of like interfaces:

Proposal C:

type autoincrement interface {
  Autoincrement()
}

type Int struct {
  value int
}

// Int implements autoincrement
func (i *Int) AutoIncrement() {
  i.value++
}

If a datatype (Int in the example above) does not implement the autoincrement interface, it's a compiler error.

Use Case Syntax (on single line due to formatting constraints)
Ok Int implements autoincrement model User { id Int @id @autoincrement }
Error String does not implement autoincrement model User { id String @id @autoincrement }

What do you think @mavilein?


@mavilein @matthewmueller what's the actionable thing that needs to happen here? What's the minimum amount of decisionmaking that unblocks engineering?

To me:

mavilein commented 4 years ago

I would like to take a step back from the discussion and first look at general approaches for solutions we could take. I would like us to first agree on one of the solution approaches. Afterwards we can talk about the details of the concrete syntax. Right now we are talking about the details without looking at the bigger picture imo.

Motivation

PSL will be used in many different scenarios. Depending on their context users will want to choose one of two behaviors for the id field of a model:

  1. Auto Generated Id: If the id field for a model is auto generated the user does not need to specify it during record creation. If no id value is provided the id will be generated either by Photon or the underlying datasource. The user still has the option to manually provide a value. This will be the prevalent choice in green field projects.

  2. Manual Id: If the id field for a model is not auto generated the user must specify a value for the id field during record creation manually. This will be needed in some cases like brownfield projects or usecases that deal with the integration between sytems.

Goals of this writeup

This writeup aims to achieve the following things:

  1. Align us internally on potential approaches for a solution and agreeing on the tradeoffs involved.
  2. Outline different syntaxes for each approach.
  3. This should provide a foundation for choosing an approach and concrete syntax.

Requirements

This a checklist for whether a solution fulfills the requirements:

  1. A solution must provide a way to define an id field with an auto generated value.
  2. A solution must provide a way to define an id field without an auto generated value.
  3. A solution must provide a pit of success for the user. The solution must guide the user to do the right thing.

Solution Approaches

This section describes possible approaches for solutions in a generic way. It does not provide syntax details but explores general tradeoffs.

1. Auto Generated is Optin, Manual is Default

We guide the user to use manual ids by making this the most concise option that requires the least amount of typing. A field that is annotated with @id does not have a auto generated value. The users has to type more to enable auto generated ids.

2. Auto Generated is Default, Manual is Optin

We guide the user to use auto generated by making this the most concise option that requires the least amount of typing. A field that is annotated with @id has a auto generated value. The users has to type more to enable manual ids.

3. No Default, PSL forces to choose behavior (both OptIn)

We do not guide the user to any of the options. Annotating a field with @id is not enough. The user has to specify whether auto generated or manual ids are used by providing the setting explicitly. Both options require roughly the same amount of typing. We can make this a lot easier by providing a nice error message and tooling support.

Summary

The approach that we take should depend on which use case we want to make easy:

Context: Behavior of the current Implementation & Problems of original Implementation

Right now we do not allow the user to disable auto generated values for id fields. We allow the following 3 definitions that all result in auto generated ids. Other definitions are forbidden:

id Int    @id
id String @id @default(cuid())
id String @id @default(uuid())

Originally the behavior was different and we treated the syntaxes id Int @id and id String @id as manual ids. However we got a lot of bug reports because people expected auto generated ids. So our original implementation followed approach 1 but that led to a lot of confusion.

Context: Relation between Solution Approach and Introspection

The introspection relies on the database schema to generate the PSL schema. It can only consider informations present in the underlying database. That means that for SQL databases it can detect whether it uses a auto generating int id. But it cannot detect whether auto generated string ids are used because the auto generation does not happen at the database level.

This has the followings implications for each of the above mentioned approaches when dealing with string ids:

mavilein commented 4 years ago

Syntax Proposals

Here are syntax proposals for the outlined approaches. These are just examples. We can add more examples and categorize them into one of those three approaches.

Syntax Proposals for Approach 1: Auto Generated is Optin, Manual is Default

The opt in for auto generated ids happens through the use of @default or @sequence/@autoincrement.

Syntax Proposals for Approach 2: Auto Generated is Default, Manual is Optin

The opt in for manual ids happens through @id(MANUAL).

Syntax Proposals for Approach 3: No Default, PSL forces to choose behavior (both OptIn)

The opt in for both approachea happens through a argument for @id.

mavilein commented 4 years ago

PS: @do4gr and I talked about this for a fairly long time yesterday and we are both in favour of approach 3.

matthewmueller commented 4 years ago

@mavilein thanks for writing your thoughts! I think we're now all in agreement that the default should be opt-in in both cases, that @id alone is not sufficient. Now we just need to come up with syntax! I'll work off of Approach 3 in the next issue (I need to think about it some more โ˜บ๏ธ)

But first, while I largely agree with your thoughts, I do want to point out a few places where my thinking differs. I'll try to speak generally to better explain my decision-making process and hopefully smooth our discussions going forward:

The users has to type more to enable auto generated ids.

I don't believe less typing means it will become the default path. I'd rather each keyword add a certain feature that's completely independent of the other keywords. If there's lots of features, there's lots of keywords and by extension lots of typing. I think that's fine.

I don't like when we have 2 different ways to do the nearly the same thing. This comes up when we talk about supporting both String @id(cuid()) and String @id @default(cuid()).

I also don't like adding keywords to turn off features. Adding keywords should only ever add features. This particular case might be an exception, because the auto-increment case far outweighs the manual case.

I do like your approach of having an enum like @id(manual()) rather than something like autoincrement(false).

So our original implementation followed approach 1 but that led to a lot of confusion.

While this is valuable initial data, we will have a far larger pool of users coming. Many of these users will come with existing SQL knowledge (SQL has a 50-year headstart). The farther we stray from SQL the more we'll need to sell the "Prisma Way". I feel much more comfortable having "SQL's blessing" as we design PSL.

But it cannot detect whether auto generated string ids are used because the auto generation does not happen at the database level.

This is something I didn't consider, thanks for bringing it up. This is very valid context that affects our proposal given our current implementation, but I think this is our self-imposed box.

We can also place uuid generation in the database. Postgres and MySQL support them natively where introspection will work best. With cuid(), we could write some user-defined function, though from my perspective cuid doesn't add anything over uuid and I'd very much like to get rid of this feature altogether.

Pushing these concepts down to the database will make our introspection better and make brownfield cases work better. It will also enable the existing SQL tooling to work in harmony with Prisma. The reality is we're going to need to do both, but where we can, I think we should rely on the database's featureset as much as possible.


matthewmueller commented 4 years ago

Reformatting Approach 3 a bit and renaming to Proposal D:

Proposal D:

Use Case Syntax (on single line due to formatting constraints)
primary key integer w/ autoincrement model User { id Int @id(autoincrement()) }
primary key integer w/o autoincrement model User { id Int @id(manual()) }
primary key string w/ default model User { id String @id(uuid()) }
primary key string w/o default model User { id String @id(manual()) }

Questions:


I'd be okay with this proposal. I don't love the word manual() and the overlap with @default, but I can commit to it nonetheless ๐Ÿ™Œ

mavilein commented 4 years ago

Is id String @id without an argument permitted?

No

Is id Int @id permitted? If so, what does it do?

No

Is id String @id(uuid()) default(cuid()) permitted?

Don't know. Forbidding this sounds like a good idea.

matthewmueller commented 4 years ago

agree with all 3 points including forbidding ๐Ÿ‘

idan commented 4 years ago

Don't know. Forbidding this sounds like a good idea. ๐Ÿ‘

I don't love the word manual() and the overlap with @default I think this is "good enough". If I'd think of different language to make it more self-evident, maybe I'd call it noauto to make the purpose clear. But I'd ship it with manual just fine.

Beyond the question of what to call manual, there's consensus around Proposal D.

Handing this off to @janpio and engineering, let product know if we can assist further.

matthewmueller commented 4 years ago

I just spoke with @divyenduz who brought another angle to the discussion. We could make the id required in Photon with:

model User {
  id String @id 
}

So photon.user.create() would be a compile error, you'd have to do photon.user.create({ id: "a" }). If you want to make id optional, you'd need default:

model User {
  id String @id @default(cuid())
}

Regarding the manual vs. autoincrement for integers, @divyenduz prefers:

Proposal E

Use Case Syntax (on single line due to formatting constraints)
primary key integer w/ autoincrement model User { id Int @id @default(autoincrement()) }
primary key integer w/o autoincrement model User { id Int @id }
primary key string w/ default model User { id String @id @default(cuid()) }
primary key string w/o default model User { id String @id }

Or Proposal F with @autoincrement

Use Case Syntax (on single line due to formatting constraints)
primary key integer w/ autoincrement model User { id Int @id @autoincrement }
primary key integer w/o autoincrement model User { id Int @id }
primary key string w/ default model User { id String @id @default(cuid()) }
primary key string w/o default model User { id String @id }

@divyenduz was also okay with Proposal A.


@matthewmueller is happy with Proposal A, Proposal D, Proposal E and Proposal F.

mavilein commented 4 years ago

@matthewmueller : Thanks for adding this valuable argument by @divyenduz . I just thought a bit more how i originally reached the conclusion that model User { id String @id } is confusing for users. We already had the behavior that @divyenduz is suggesting. We were giving a compile error in this case (i just checked the DMMF code). However users were not able to link back the error (required argument 'id' is missing) to the fact they should make the id autogenerated by adding @default.

I see two approaches to address this confusion: a. Improve the photon error message by providing the user with a hint how this could be fixed. E.g.: Please supply the missing argument for 'id'. If you want to use auto generated ids add '@default(uuid)' or '@default(cuid())' to the id field. b. Make this more explicit by making manual ids opt in.

I did not consider approach a so far to be honest. I see two problems with it though:

matthewmueller commented 4 years ago
  • Providing such a nice compile error in Typescript i possible i guess. However in Go this will not be possible. This applies to most other languages as well.
  • It seems a bit weird to educate the user about a data modeling mistake with a compiler error in Photon.

I agree with both of these points, but I think they're pretty easy hurdles to overcome. Despite the error being unhelpful, you'll find out about your mistake during development and I think this is the type of mistake that would make sense once you figure it out.


I like Proposal E the most because it makes it clear that autoincrement() is a default that can be overridden in photon.users.create(...)

sorenbs commented 4 years ago

It seems that our thoughts on this topic are converging. I will attempt to capture all facets in a single piece of writing that we can all sign off on, and use as the basis for a pr to the schema spec.

Relations

Relations is a high-level construct in Prisma and a key value proposition compared to other data access libraries. The relation construct is a convenient way to apply the common pattern of using keys stored in one or more fields in a model to point to a specific record of another model.

Explicit relation The relation can be set up explicitly by defining the referenced field:

Note: the syntax below is omitting any details not related to the concept it explains

model User {
    name String
}

model Post {
    author User @relation(references: name)
}

Implicit relation If the referenced model has a primary identifier stored in one or more fields, the field(s) do not need to be provided explicitly:

model User {
    name String @id
}

model Post {
    author User
}

@id

The @id attribute is a shorthand for the relation construct as described above.

Additionally it has two special meanings:

the field is not nullable.

Specifically, the following syntax is not allowed:

model User {
    name String? @id
}

Implementation Note: Not all connectors can provide the non-null guarantee, so the Query Engine must gracefully handle records with null-valued ids. Details to be mapped out in the spec pr.

the field is unique

No additional syntax is required for marking the field unique. Using both @id and @unique together is disallowed:

model User {
    name String @id @unique
}

Implementation Note: The Query Engine will rely on the connector providing this guarantee, and will not attempt to check for uniqueness. Not all connectors can provide the unqueness guarantee however, so the Query Engine must gracefully handle multiple records with the same id. It seems reasonable to throw an error if that condition is encountered, details to be mapped out in the spec pr.

@default

It is possible to specify a default value that the Query Engine will insert:

static default value

model User {
    name String @id @default("Bob")
}

When a default value is specified,the user can either omit or provide a value for the field when creating a record. If the field value is omitted, the default value is added by the Query Engine before the record is persisted in the database.

dynamic default value

A static default value is not very useful for @id fields as they are unique. Prismas Query Engine provides a number of dynamic default values:

model User {
    name String @id @default(cuid())
}

Potential dynamic default values:

In the future we might support arbitrary expressions for default values.

database assisted default values

Some default values schemes can not be implemented efficiently in the Query Engine. To address this we establish a simple agreement between the Query Engine and the database (meaning the dba or any prisma-aware migration tool such as Lift). Under this agreement, the database will do all the work, and the Query Engine will trust the database to do the right thing.

model User {
    name String @id @default(autoincrement())
}

Potential database assisted default values:

matthewmueller commented 4 years ago

Thanks for writing this up! I think it does a good job of boiling down the discussion.

Some thoughts on @default:

It's not clear to me that autoincrement() and dbProvided() are mutually exclusive. I can imagine a world where we'd provide Prisma-level sequences using an atomic counter. In that world we'd have @default(autoincrement()) where autoincrement() is not database-assisted.

@default(dbProvided()) would not allow us to spin up a replica of an existing database from a Prisma schema, since we've lost the database's default expressions.

A couple additional questions that popped into my head:


Everything else +๐Ÿ’ฏ from me!

schickling commented 4 years ago

Looks mostly good from my side as well. ๐Ÿ‘

My biggest point of feedback would be that we should try to draw a clearer line (both in terms of concepts and syntax) when a feature is implemented on the Prisma level vs when a DB feature is used (See #305).

sorenbs commented 4 years ago

Thanks Matt!

As I answer each question separately, keep in mind that we are working towards a launch of Photon in early Q1 2020, and explicitly accepting that some questions will have to be left unanswered for the time being.

1) I can imagine a world where we'd provide Prisma-level sequences using an atomic counter.

This is far in the future, and we will have to revisit the schema when we introduce this feature

2) @default(dbProvided()) would not allow us to to spin up a replica of an existing database from a Prisma schema

This is accurate. We will need to introduce extra syntax when we work towards the Lift release following the Photon release.

3) Is it possible to have @default(dbProvided()) on something other than an @id?

Yes. The presence of a database assisted default value on a required field simply makes the input field optional.

4) What does the transition look like to migrate from @default(uuid()) to @default(dbProvided())? How about the other way?

If you have a schema like this

model User {
  id String @default(uuid())
}

and change it to this:

model User {
  id String @default(dbProvided())
}

then queries that create a User without explicitly setting the id will fail.

Once you migrate the database to actually have a default value for the id field, the query will start working again. You can perform this transition without disrupting your application by migrating the database first.

5) Do you anticipate a future where we transition to native default values for databases that support it?

I think this will be a different syntax. We will not explore this further before the initial Photon release.

sorenbs commented 4 years ago

We have reached a decision on this issue.

I will transform it into a Spec PR with the aim to have it merged early next week.

@mavilein and @janpio - we can begin implementation.

do4gr commented 4 years ago

One question that is not defined in the issue:

Our old behaviour was that Int ids are auto incrementing and therefore cannot be provided by the user and are not part of the CreateInputType therefore. For CUIDs and UUIDs the user could provide them, even when they were defined as default (and only UUIDs were validated I think). Do we want to keep this behaviour? It seems not consistent atm.

id Int @id @default(autoincrement()) # you cannot set it yourself
id String @id @default(cuid())       # you can provide whatever string you want yourself
id String @id @default(uuid())       # you can provide a valid UUID yourself
sorenbs commented 4 years ago

Do we want to keep this behaviour? It seems not consistent atm.

No. We want it to be consistent. You should be able to provide your own id in all cases. Later we will introduce a way to make a field read-only, but that is orthogonal to this issue.

schickling commented 4 years ago

Related to this we also thought of introducing a @default(autoincrement(), writeable: false) feature in this issue.

janpio commented 4 years ago

What is left to do here @mavilein ?

mavilein commented 4 years ago

This is done.