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

feat(validation): disallow SetNull referential action on required referenced fields #3298

Merged
merged 13 commits into from Oct 20, 2022

Conversation

jkomyno
Copy link
Contributor

@jkomyno jkomyno commented Oct 17, 2022

This PR validates against required fields referenced by a relation with at least one SetNull referential action, regardless of whether it appears in onUpdate or onDelete.

E.g., with the following schema,

model SomeUser {
  id      Int      @id
  ref     Int
  profile Profile?
  @@unique([id, ref])
}

model Profile {
  id       Int       @id
  user     SomeUser? @relation(fields: [user_id, user_ref], references: [id, ref], onDelete: SetNull)
  user_id  Int?
  user_ref Int
  @@unique([user_id, user_ref])
}

this PR introduces the following validation error:

The onDelete referential action of a relation must not be set to SetNull when a referenced field is required.
Either choose another referential action, or make the referenced fields optional.

Note: the only SQL database we support which would accept the DDL declaration equivalent to the Prisma schema above is Postgres, although it would still fail at runtime when the SET NULL referential action is triggered.
This PR, however, introduces this new validation regardless of the used database connector. See internal doc for more details. It should be noted that Tom has expressed some concerns on validating against something that Postgres actually supports (although we argue that Postgres' support in this case is a bug).

Closes prisma/prisma#14673.

@jkomyno jkomyno changed the title test: add skeleton for prohibiting SetNull ref actions referencing scalar fields feat(validation): disallow SetNull referential action on optional referenced fields Oct 17, 2022
@jkomyno jkomyno changed the title feat(validation): disallow SetNull referential action on optional referenced fields feat(validation): disallow SetNull referential action on required referenced fields Oct 19, 2022
@jkomyno jkomyno added this to the 4.6.0 milestone Oct 19, 2022
…ehavior in 'referential_actions_are_kept_intact' test
Copy link
Member

@Jolg42 Jolg42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me though, let's confirm with someone with more knowledge in the engines.

@jkomyno jkomyno marked this pull request as ready for review October 20, 2022 10:01
@jkomyno jkomyno requested a review from a team as a code owner October 20, 2022 10:01
Copy link
Contributor

@pimeys pimeys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nitpicks, also somebody from the @prisma/client-team-rust should ack the removed tests.

Copy link
Member

@Weakky Weakky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed tests make sense to me considering the new validation rule 👍

@Jolg42
Copy link
Member

Jolg42 commented Oct 20, 2022

Note: let's monitor introspection-ci later

@jkomyno jkomyno merged commit abd71fc into main Oct 20, 2022
@jkomyno jkomyno deleted the fix/set-null-validation branch October 20, 2022 12:32
@Jolg42
Copy link
Member

Jolg42 commented Oct 25, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation fails to detect invalid SetNull referential action referencing non-optional fields
4 participants