Referential Integrity setting to the PSL #2212

pimeys · 2021-09-03T13:09:31Z

Changes datasource configuration to hold a setting for referential integrity for relations defined in the data model. At the same time removes the preview feature of planetScaleMode and removes the setting for the planetScaleMode in the datasource.

The new format works like so:

datasource db {
  provider = "mysql"
  referentialIntegrity = "prisma"
  url = "mysql://root:prisma@localhost:3306/mydb"
}

generator client {
  provider = "prisma-client-js"
  previewFeatures = ["referentialIntegrity"]
}

model A {
  id Int @id
  bs B[]
}

model B {
  id Int @id
  aId Int
  a A @relation(fields: [aId], references: [id], onDelete: Restrict, onUpdate: SetNull)
}

In this case, we connect to mysql database, but instead of creating a foreign keys, we handle the referential integrity in the Query Engine. This is useful on databases where one cannot use foreign keys due to scaling issues, or probably more commonly for our users in PlanetScale.

The allowed values for this setting are either:

prisma, that handles the integrity in the Query Engine. This is the default for MongoDB.
foreignKeys, that lets the database handle the integrity using foreign key constraints. This is the default for all SQL databases.

On SQL databases both settings are allowed. On Mongo, only prisma setting is allowed.

The migration and introspection engine tests are modified to run in the prisma mode with Vitess. Re-introspection will still remove relations from the PSL if they are not in the database as foreign keys.

Partially resolves prisma/prisma#7293

tomhoule

Looks great!

We discussed the two problems I see (conditional connectors, ME test setup woes), and they will be addressed later.

Maybe @dpetrick should review the QE test setup changes?

tomhoule · 2021-09-13T09:06:56Z

.buildkite/engineer

@@ -13,7 +13,7 @@ fi
 if ! type "engineer" > /dev/null; then
    # Setup Prisma engine build & test tool (engineer).
    set -e
-    curl --fail -sSL "https://prisma-engineer.s3-eu-west-1.amazonaws.com/1.22/latest/$OS/engineer.gz" --output engineer.gz
+    curl --fail -sSL "https://prisma-engineer.s3-eu-west-1.amazonaws.com/remove-cargo-tests/latest/$OS/engineer.gz" --output engineer.gz


Is this intended to be kept?

Nope. Releasing a new version.

tomhoule · 2021-09-13T09:08:17Z

.github/workflows/datamodel.yml

+            target
+          key: datamodel-cache
+
+      - run: cargo test -p datamodel -p dml -p datamodel-connector -p sql-datamodel-connector


I remember we talked about it but can't remember the details. If we only test these, it still means some crates won't have their tests run in CI, and by default, when we add a new crate with tests, it won't be in CI. Why don't we cargo test everything, just excluding the crates that need live databases and have their own workflows?

We'd like to have separate runs for separate test projects. This one tests the datamodel, if one introduces new tests, their responsibility is to either run them in Buildkite or GitHub.

I checked and the only tests that we were running on cargo tests were these.

tomhoule · 2021-09-13T09:09:10Z

docker-compose.yml

@@ -35,7 +35,7 @@ services:
      - "5431:5432"
    networks:
      - databases
-    tmpfs: /pgtmpfs9
+    tmpfs: /pgtmpfs9:size=4g


That was added back but I think we shouldn't have the tmpfs at all. Maybe a discussion for the weekly rust meeting?

migration-engine/migration-engine-tests/tests/migrations/sql.rs

tomhoule · 2021-09-13T09:16:14Z

migration-engine/connectors/sql-migration-connector/src/sql_schema_calculator.rs

@@ -24,7 +24,7 @@ pub(crate) fn calculate_sql_schema(
    let mut relation_tables: Vec<_> = calculate_relation_tables(datamodel, flavour, &schema).collect();
    schema.tables.append(&mut relation_tables);

-    if configuration.planet_scale_mode() {
+    if !configuration.referential_integrity().uses_foreign_keys() {


tomhoule · 2021-09-13T09:17:12Z

libs/datamodel/core/src/common/preview_features.rs

@@ -54,7 +55,7 @@ features!(
    SelectRelationCount,
    OrderByAggregateGroup,
    FilterJson,
-    PlanetScaleMode,
+    ReferentialIntegrity,


That means we'll break people using the planetScaleMode preview feature. It's preview so I'm ok with that.

We'll have to make sure to mention it in the release notes.

libs/datamodel/core/src/common/preview_features.rs

tomhoule · 2021-09-13T09:30:58Z

libs/datamodel/connectors/sql-datamodel-connector/src/lib.rs

-    pub fn postgres() -> PostgresDatamodelConnector {
-        PostgresDatamodelConnector::new()
+    pub fn postgres(referential_integrity: ReferentialIntegrity) -> PostgresDatamodelConnector {
+        PostgresDatamodelConnector::new(referential_integrity)


We had a slack conversation about this, and we agree it's debt we take on now, but we have to revisit how this works later, because it's not where we want to be.

migration-engine/migration-engine-tests/tests/migrations/basic/vitess.rs

tomhoule · 2021-09-13T09:41:52Z

libs/datamodel/connectors/datamodel-connector/src/referential_integrity.rs

+            ReferentialIntegrity::Prisma => write!(f, "prisma"),
+        }
+    }
+}


Adds a new parameter to the PSL `datasource` block. `referentialIntegrity` takes a string value, either `prisma` or `foreignKeys`. The value `prisma` handles referential integrity on the Prisma level, and `foreignKeys` in the database using foreign key constraints.

Setting the Vitess docker images to prevent using foreign keys, emulating the behavior of PlanetScale, so we can carefully select which Migration Engine tests we should run, setting them to not create foreign keys, and then disabling the tests which try to read foreign keys from the database asserts.

Again, as with Migration Engine tests, selecting the prominent tests from Introspection and Query Engines, that they work nicely and disabling the ones that expect to see relations. For now this means we cannot introspect relations, and all tests using them are disabled.

dpetrick

For QE part.

pimeys marked this pull request as draft September 3, 2021 13:09

pimeys force-pushed the migration-engine/no-foreign-key-mode branch 4 times, most recently from 62c3483 to 3d66c3c Compare September 3, 2021 15:11

pimeys marked this pull request as ready for review September 6, 2021 11:31

pimeys added this to the 3.1.0 milestone Sep 7, 2021

pimeys force-pushed the migration-engine/no-foreign-key-mode branch 2 times, most recently from 62555d2 to af76394 Compare September 7, 2021 19:07

tomhoule reviewed Sep 13, 2021

View reviewed changes

Julius de Bruijn added 3 commits September 13, 2021 14:20

pimeys force-pushed the migration-engine/no-foreign-key-mode branch from af76394 to bee4b32 Compare September 13, 2021 12:20

dpetrick approved these changes Sep 13, 2021

View reviewed changes

Engineer 1.23

f31a530

pimeys merged commit 0cecbd5 into master Sep 13, 2021

pimeys deleted the migration-engine/no-foreign-key-mode branch September 13, 2021 12:55

pimeys mentioned this pull request Sep 13, 2021

Document referential integrity feature prisma/docs#2317

Closed

davesteinberg mentioned this pull request Sep 14, 2021

Support (some) weak relations with no foreign key prisma/prisma#7351

Open

pimeys mentioned this pull request Sep 14, 2021

Support databases without foreign keys prisma/prisma#7293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Referential Integrity setting to the PSL #2212

Referential Integrity setting to the PSL #2212

pimeys commented Sep 3, 2021 •

edited by janpio

tomhoule left a comment

tomhoule Sep 13, 2021

pimeys Sep 13, 2021

tomhoule Sep 13, 2021

pimeys Sep 13, 2021

tomhoule Sep 13, 2021

tomhoule Sep 13, 2021

tomhoule Sep 13, 2021

tomhoule Sep 13, 2021

tomhoule Sep 13, 2021

pimeys Sep 13, 2021

tomhoule Sep 13, 2021

dpetrick left a comment

Referential Integrity setting to the PSL #2212

Referential Integrity setting to the PSL #2212

Conversation

pimeys commented Sep 3, 2021 • edited by janpio

tomhoule left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpetrick left a comment

Choose a reason for hiding this comment

pimeys commented Sep 3, 2021 •

edited by janpio