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

Validation fails to detect invalid SetNull referential action referencing non-optional fields #14673

Closed
Tracked by #11441
jkomyno opened this issue Aug 5, 2022 · 7 comments · Fixed by prisma/prisma-engines#3298

Comments

@jkomyno
Copy link
Contributor

jkomyno commented Aug 5, 2022

This missed validation triggers a migration error when using MySQL, SQL Server, SQLite, and CockroachDB, but not on Postgres.

Example with MySQL:

// schema.prisma

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

datasource db {
  provider = "mysql"
  url = env("DATABASE_URI_MYSQL") 
}

model User {
  id      String @id
  profile Profile?
}

model Profile {
  id       String @id
  user     User @relation(fields: [userId], references: [id], onUpdate: SetNull, onDelete: SetNull)
  
  // notice that this field should become optional in order to support `SetNull`
  userId   String @unique
}

We can see that the schema is wrongfully considered valid:

❯  prisma validate
Prisma schema loaded from prisma/schema.prisma
The schema at /.../reprod/prisma/schema.prisma is valid 🚀

If we attempt a push, we get the following error:

❯ prisma db push --skip-generate
Prisma schema loaded from prisma/schema.prisma
Datasource "db": MySQL database "PRISMA_DB_NAME" at "localhost:3306"

MySQL database PRISMA_DB_NAME created at localhost:3306
Error: Column 'userId' cannot be NOT NULL: needed in a foreign key constraint 'Profile_userId_fkey' SET NULL
   0: sql_migration_connector::apply_migration::migration_step
           with step=AddForeignKey { foreign_key_id: ForeignKeyId(0) }
             at migration-engine/connectors/sql-migration-connector/src/apply_migration.rs:21
   1: sql_migration_connector::apply_migration::apply_migration
             at migration-engine/connectors/sql-migration-connector/src/apply_migration.rs:10
   2: migration_core::state::SchemaPush
             at migration-engine/core/src/state.rs:384

If we try to create/update the Profile model via the Prisma client, we get the following migration error:

await prisma.$transaction([
  prisma.user.create({
    data: {
      id: '1'.
      profile: {
        create: { id }
      }
    }
  })
])
    Column 'userId' cannot be NOT NULL: needed in a foreign key constraint 'Profile_userId_fkey' SET NULL
       0: sql_migration_connector::apply_migration::apply_migration
                 at migration-engine/connectors/sql-migration-connector/src/apply_migration.rs:10
       1: migration_core::state::SchemaPush
                 at migration-engine/core/src/state.rs:384

If we exclude the onUpdate/onDelete referential actions, the schema above generates the following SQL statements (we can check that via prisma migrate dev):

-- CreateTable
CREATE TABLE `User` (
    `id` VARCHAR(191) NOT NULL,

    PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;

-- CreateTable
CREATE TABLE `Profile` (
    `id` VARCHAR(191) NOT NULL,
    `userId` VARCHAR(191) NOT NULL,

    UNIQUE INDEX `Profile_userId_key`(`userId`),
    PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;

-- AddForeignKey
ALTER TABLE `Profile` ADD CONSTRAINT `Profile_userId_fkey` FOREIGN KEY (`userId`) REFERENCES `User`(`id`);

On Postgres, oddly, prisma db push it doesn't fail, so migration error is thrown:

❯ prisma db push --skip-generate
Prisma schema loaded from prisma/schema.prisma
Datasource "db": PostgreSQL database "PRISMA_DB_NAME", schema "public" at "localhost:5432"

PostgreSQL database PRISMA_DB_NAME created at localhost:5432

🚀  Your database is now in sync with your Prisma schema. Done in 64ms

SetNull on Valid Schema

If we want prisma db push to work on other databases like mysql:8 when using the SetNull referential action, we'd need to change the schema above as:

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

datasource db {
  provider = "mysql"
  url = env("TEST_MYSQL_URI_MIGRATE")
  referentialIntegrity = "foreignKeys"
}

model User {
  id      String @id
  profile Profile?
}

model Profile {
  id       String @id
  user     User? @relation(fields: [userId], references: [id], onUpdate: SetNull, onDelete: SetNull)
  userId   String? @unique
}

which generates the following SQL statements:

-- CreateTable
CREATE TABLE `User` (
    `id` VARCHAR(191) NOT NULL,

    PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;

-- CreateTable
CREATE TABLE `Profile` (
    `id` VARCHAR(191) NOT NULL,
    `userId` VARCHAR(191) NULL,

    UNIQUE INDEX `Profile_userId_key`(`userId`),
    PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;

-- AddForeignKey
ALTER TABLE `Profile` ADD CONSTRAINT `Profile_userId_fkey` FOREIGN KEY (`userId`) REFERENCES `User`(`id`) ON DELETE SET NULL ON UPDATE SET NULL;
@tomhoule
Copy link
Contributor

tomhoule commented Oct 5, 2022

Is this actually not valid at the database level? We should test first that a CREATE TABLE exhibiting that pattern is rejected by the databases we support before writing a PSL validation.

@jkomyno jkomyno self-assigned this Oct 6, 2022
@jkomyno
Copy link
Contributor Author

jkomyno commented Oct 6, 2022

SQL Reproduction with one-to-many non-optional relations

MySQL

Test SQL Statement:

CREATE TABLE categories(
  categoryId INT AUTO_INCREMENT PRIMARY KEY,
  categoryName VARCHAR(100) NOT NULL
) ENGINE=INNODB;

CREATE TABLE products(
  productId INT AUTO_INCREMENT PRIMARY KEY,
  productName varchar(100) not null,
  categoryId INT,
  CONSTRAINT fk_category
  FOREIGN KEY (categoryId) 
      REFERENCES categories(categoryId)
      ON DELETE ${action}
      ON UPDATE ${action}
) ENGINE=INNODB;

with

type Action
  = 'RESTRICT'
  | 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on mysql:5.7 and mysql:8:

  • RESTRICT
  • CASCADE
  • SET NULL
  • NO ACTION

The following only works on mysql:8:

  • SET DEFAULT

On mysql:5.7, it fails with:

SqlError: (conn=22, no: 1215, SQLState: HY000) Cannot add foreign key constraint

MariaDB

Test SQL Statement:

CREATE TABLE categories(
  categoryId INT AUTO_INCREMENT PRIMARY KEY,
  categoryName VARCHAR(100) NOT NULL
) ENGINE=INNODB;

CREATE TABLE products(
  productId INT AUTO_INCREMENT PRIMARY KEY,
  productName varchar(100) not null,
  categoryId INT,
  CONSTRAINT fk_category
  FOREIGN KEY (categoryId) 
      REFERENCES categories(categoryId)
      ON DELETE ${action}
      ON UPDATE ${action}
) ENGINE=INNODB;

with

type Action
  = 'RESTRICT'
  | 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on mariadb:10.0 and mariadb:10:

  • RESTRICT
  • CASCADE
  • SET NULL
  • NO ACTION

The following only works on mariadb:10:

  • SET DEFAULT

On mariadb:10.0, it fails with:

SqlError: (conn=15, no: 1005, SQLState: HY000) Can't create table `products` (errno: 150 "Foreign key constraint is incorrectly formed")

Postgres / CockroachDB

Test SQL Statement:

CREATE TABLE categories(
  categoryId INT PRIMARY KEY,
  categoryName VARCHAR(100) NOT NULL
);

CREATE TABLE products(
  productId INT PRIMARY KEY,
  productName varchar(100) not null,
  categoryId INT,
  CONSTRAINT fk_category
  FOREIGN KEY (categoryId) 
      REFERENCES categories(categoryId)
      ON DELETE ${action}
      ON UPDATE ${action}
);

with

type Action
  = 'RESTRICT'
  | 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on postgres:10, postgres:14, prismagraphql/cockroachdb-custom:22.1.0:

  • RESTRICT
  • CASCADE
  • SET NULL
  • NO ACTION
  • SET DEFAULT

Microsoft SQL Server

Test SQL Statement:

CREATE TABLE categories(
  categoryId INT NOT NULL PRIMARY KEY NONCLUSTERED,
  categoryName NVARCHAR(100) NOT NULL
);

CREATE TABLE products(
  productId INT NOT NULL PRIMARY KEY NONCLUSTERED,
  productName NVARCHAR(100) NOT NULL,
  categoryId INT,
  CONSTRAINT fk_category
  FOREIGN KEY (categoryId) 
      REFERENCES categories(categoryId)
      ON DELETE ${action}
      ON UPDATE ${action}
);

with

type Action
  = 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on mcr.microsoft.com/azure-sql-edge:latest:

  • CASCADE
  • SET NULL
  • NO ACTION
  • SET DEFAULT

@jkomyno
Copy link
Contributor Author

jkomyno commented Oct 6, 2022

SQL Reproduction with one-to-one optional relations

MySQL

Test SQL Statement:

CREATE TABLE User (
    id INTEGER PRIMARY KEY
) ENGINE=InnoDB;

CREATE TABLE Profile (
    id INTEGER PRIMARY KEY,
    userId INTEGER UNIQUE NULL,
    FOREIGN KEY (userId) REFERENCES User(id)
      ON DELETE ${action}
      ON UPDATE ${action}
) ENGINE=InnoDB;

with

type Action
  = 'RESTRICT'
  | 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on mysql:5.7 and mysql:8:

  • RESTRICT
  • CASCADE
  • SET NULL
  • NO ACTION

The following only works on mysql:8:

  • SET DEFAULT

On mysql:5.7, it fails with:

SqlError: (conn=22, no: 1215, SQLState: HY000) Cannot add foreign key constraint

MariaDB

Test SQL Statement:

CREATE TABLE User (
    id INTEGER PRIMARY KEY
) ENGINE=InnoDB;

CREATE TABLE Profile (
    id INTEGER PRIMARY KEY,
    userId INTEGER UNIQUE NULL,
    FOREIGN KEY (userId) REFERENCES User(id)
      ON DELETE ${action}
      ON UPDATE ${action}
) ENGINE=InnoDB;

with

type Action
  = 'RESTRICT'
  | 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on mariadb:10.0 and mariadb:10:

  • RESTRICT
  • CASCADE
  • SET NULL
  • NO ACTION

The following only works on mariadb:10:

  • SET DEFAULT

On mariadb:10.0, it fails with:

SqlError: (conn=15, no: 1005, SQLState: HY000) Can't create table `products` (errno: 150 "Foreign key constraint is incorrectly formed")

Postgres / CockroachDB

Test SQL Statement:

CREATE TABLE User (
    id INT PRIMARY KEY
);

CREATE TABLE Profile (
    id INT PRIMARY KEY,
    userId INT UNIQUE NULL,
    FOREIGN KEY (userId) REFERENCES User(id)
      ON DELETE ${action}
      ON UPDATE ${action}
);

with

type Action
  = 'RESTRICT'
  | 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on postgres:10, postgres:14, prismagraphql/cockroachdb-custom:22.1.0:

  • RESTRICT
  • CASCADE
  • SET NULL
  • NO ACTION
  • SET DEFAULT

Microsoft SQL Server

Test SQL Statement:

CREATE TABLE SomeUser (
  id INT PRIMARY KEY NONCLUSTERED
);

CREATE TABLE Profile (
  id INT PRIMARY KEY NONCLUSTERED,
  userId INT UNIQUE NULL,
  FOREIGN KEY (userId) REFERENCES SomeUser(id)
    ON DELETE ${action}
    ON UPDATE ${action}
);

with

type Action
  = 'CASCADE'
  | 'SET NULL'
  | 'NO ACTION'
  | 'SET DEFAULT'

let action: Action

The following SQL referential actions work for both ON UPDATE and ON DELETE on mcr.microsoft.com/azure-sql-edge:latest:

  • CASCADE
  • SET NULL
  • NO ACTION
  • SET DEFAULT

@jkomyno
Copy link
Contributor Author

jkomyno commented Oct 6, 2022

To recap, @tomhoule, the SQL database don't validate the CREATE TABLE statements when defining SET NULL referential actions, but I think Prisma should, in order to avoid panics at runtime.

@tomhoule
Copy link
Contributor

tomhoule commented Oct 6, 2022

but I think Prisma should, in order to avoid panics at runtimes.

Makes sense. One thing to keep in mind that could prevent us from going into that direction is the principle that we should be able to introspect a valid database schema to a valid prisma schema in as many cases as possible, so if it's valid at the database level, we may want to accept it. Maybe warn.

@Jolg42
Copy link
Member

Jolg42 commented Oct 11, 2022

What we want to do (from Notion)

Make schema with non-optional 1:1 relation and SetNull referential actions invalid.

Optional but nice DX: Add special error message for PostgreSQL that links to a documentation page that explains why some valid SQL for PostgreSQL can lead to an invalid Prisma schema via Introspection: While the CREATE TABLE works, the INSERT/UPDATE/DELETE on that table will all fail.

@jkomyno
Copy link
Contributor Author

jkomyno commented Oct 11, 2022

To summarise the output of the tests done in #15728 relevant to this issue:

  • For both 1:1 and 1:n relations:
    • postgres doesn't validate DDL statements that set a SET NULL referential action on a foreign key constraint whose referenced column (or one of the referenced columns) is NOT NULL, all other database throw a validation error in this case.
    • postgres fails at runtime with a not-null constraint violation error when UPDATE / DELETE statements trigger the SET NULL referential action on a column defined as NOT NULL

Optional but nice DX: There's also an odd behavior that may be useful to document and/or validate against:

  • sqlserver fails at runtime on 1:1 NULL relations because NULL conflicts with UNIQUE indexes in that particular database

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