specs icon indicating copy to clipboard operation
specs copied to clipboard

Type Mapping – Custom Data Type Improvements

Open matthewmueller opened this issue 6 years ago • 5 comments

Problem

This proposal aims to loosen our core data type guarentee and cleanup custom data types in our schema design:

1. Not every datasource will support every core data type

Today we define the following Core Data Type: String, Float, Int, DateTime, Boolean.

These are datatypes we guarantee to work across all of our datasources. By offering core data types, you can provide a simple mental model to Prisma users and fluidly move between databases. We've implemented core data types today and they work decently[1] across Postgres, SQLite, Mongo and MySQL.

Baked into this argument is the assumption that every datasource has a concept of a String, Float, Int, DateTime, Boolean. This already breaks for a data source like Memcached. Memcached only supports a String type. We can always work around these issues (by stuffing everything into a String), but we are doing so at the expense of performance and future-features (e.g. extract the day from DateTime).

I have no doubt we'll continue to struggle with this decision as we consider adding new core data types and expand our datasource offering.

2. Cleanup edge cases & confusion around type attributes

This is mostly aestetic, but our schema today allows us to "upgrade" the type by adding @datasource.TYPE to a type definition. For example,

type Citext = String @pg.Citext
type Money = String @pg.Money

This works okay, but in many cases it doesn't make sense when consider type upgrades with other attributes:

type UUID = String @pg.UUID @pg.Text
type Float8 = Float @pg.Float8 @default("abc")
type Money = String @pg.Money // or Int? it's not clear to me.

I could see this being a common source of confusion. We can lint for these errors, but why have it in the first place?

Another area for syntax cleanup is that we already encode pg.UUID maps to a String inside the connector, so we don't need the user to manually do this in their schema.

Proposed Solution

The proposed solution is to loosen our guarantee that every data sources we ship provides a single set of core data types. Instead we make a best-effort to provide a common set of data types. In this proposal, we call these Standard Data Types.

From engineering perspective, it means we cannot bake in the assumption of Core Data Types, we need to be looser with our implementation.

Footnotes

[1] https://github.com/prisma/lift/issues/116 exposes cracks. We can solve this with our current schema spec though.

matthewmueller avatar Oct 29 '19 10:10 matthewmueller

I think this spec is not considering the right layers in the query path. In a query we always have 3 layers of types involved:

  1. underlying database type (represented through @raw)
  2. the type in the protocol between query engine and photon client (represented through @transport)
  3. the type exposed in the generated photon client

The 3rd layer is missing in this specification in my opinion. Omitting this information leads to an incomplete schema. Here's an example

type Text @raw("text") @transport("[]byte")

If i use this type in a model it is clear that the underlying database type will be text. It will be transported as an array of bytes according to this definition. But what is the exposed type in the photon client?

Based on this understanding i suggest the following changes:

  • I as a user don't care at all about transport level type. This should not be part of the spec imo. I don't gain anything by knowing this. This would significantly complicate the implementations of connectors and photon clients. At the same time the query engine would lose important semantic information and would not be able to perform validations, e.g. is a value actually a valid DateTime?
  • I as a user want to know which type will be exposed in the client. I would specify that through something like @photon(String) where the argument are our old core data types as a first step.

TLDR: I think we should consider the two layers database and photon in our type mappings.

mavilein avatar Oct 29 '19 15:10 mavilein

The 3rd layer is missing in this specification in my opinion. Omitting this information leads to an incomplete schema. Here's an example

Good call, strongly agree. In fact, I think 2) is less important than 3). Thoughts @schickling?

I'm very much in favor of coming back to this after Prisma 2 is out the door. I also think if we do 3) we've done 2)

matthewmueller avatar Oct 31 '19 16:10 matthewmueller

updated the spec based on feedback and added an illustration. the terminology is a first-draft and we're not sure what / where / if things like @go("decimal.Decimal") should be possible. But we need to consider it. Thanks @mavilein for surfacing that.

@schickling @idan please additionally motivate this change as you see fit.

matthewmueller avatar Nov 01 '19 12:11 matthewmueller

Just to add to this one: is the implementation of this spec expected to be part of Prisma2 GA?

Without at least for example TEXT, I find it hard to see how Lift could be used by any non-trivial real world usecase?

robmurtagh avatar Nov 25 '19 11:11 robmurtagh

Any update on this issue..?

kevinOriginal avatar Mar 29 '20 11:03 kevinOriginal