feat(psl): remove 'SetDefault' support on MySQL #3363

jkomyno · 2022-11-04T14:54:45Z

See the internal document for reference.

Validation message:

Invalid referential action: SetDefault. Allowed values: (Cascade, Restrict, NoAction, SetNull). SetDefault is invalid for MySQL when using relationMode = "foreignKeys", as MySQL does not support the SET DEFAULT referential action.
Learn more at prisma/prisma#11498

TODO:

test validation
test introspection
add clear final comment to MySQL does not support onDelete: setDefault prisma#11498
- add link to pris.ly

Contributes to prisma/prisma#11498.

janpio · 2022-11-04T17:04:18Z

psl/psl-core/src/validate/validation_pipeline/validations/relation_fields.rs

+        let additional_info = match (action, connector.provider_name()) {
+            (ReferentialAction::SetDefault, "mysql") => Some(formatdoc! {
+                r#"
+                    . `{set_default}` is invalid for {provider} when using `relationMode = \"foreignKeys\"`, as {provider} does not support the `SET DEFAULT` referential action.


This also happens when not relationMode is set. Can we take that into account in the error message wording?

Also: MySQL docs say it does support it. How can we make this clearer? "... does not actually support properly ..." is probably a bit too salty :D

Technically the complementary of relationMode = "prisma" is indeed relationMode = "foreignKeys". What's your suggestion here? Should I just get rid of the when using... part?

Help with the message wording is welcomed :)

No, for the user there is no relationMode at all in use in most cases.

This could work:
"when using" => "(by default or when using relationMode=foreignKeys)"

I don't think we should have a specific message for this, it reads like something generic and it should be (at first sight).

How about:

Invalid referential action: SetDefault. Allowed values: (Cascade, Restrict, NoAction, SetNull). Starting from Prisma v4.6.0, we disabled SetDefault on MySQL (when using relationMode = "foreignKeys" or when not using referential integrity at all), as MySQL does not natively support the SET DEFAULT referential action. Learn more at ..."

that's fine

We should not include version information in error messages.

The second bit of the error message is because we differ from what MySQL supports there @tomhoule. People can have this in their database, and after Introspection they will get this and then get a validation error. This second sentence gives the necessary context to be able to understand and then links to documentation that goes deeper into that.

(We'll talk about this in our team sync today)

…ysql:8, added comment

janpio · 2022-11-04T17:06:40Z

psl/psl-core/src/validate/validation_pipeline/validations/relation_fields.rs

+            (ReferentialAction::SetDefault, "mysql") => Some(formatdoc! {
+                r#"
+                    . `{set_default}` is invalid for {provider} when using `relationMode = \"foreignKeys\"`, as {provider} does not support the `SET DEFAULT` referential action.
+                    Learn more at https://github.com/prisma/prisma/issues/11498


This should already be a pris.ly link that redirects to this URL, so we can fix this later without a release. Jsut create the shortlink by adding to the vercel config file there via a PR.

I haven't ever done that, could you please post here a link to a pris.ly README or something? I'll address that on Monday.

Here: https://github.com/prisma/pris.ly/blob/main/vercel.json

tomhoule · 2022-11-07T08:41:43Z

introspection-engine/introspection-engine-tests/tests/referential_actions/mysql.rs

+    expected_validation.assert_eq(&validation.err().unwrap());
+
+    Ok(())
+}


We've always strived to make introspected output be valid for any existing database, this is a departure.

psl/psl/tests/attributes/relations/referential_actions.rs

tomhoule · 2022-11-07T09:51:29Z

psl/psl-core/src/validate/validation_pipeline/validations/relation_fields.rs

@@ -151,8 +152,21 @@ pub(super) fn referential_actions(field: RelationFieldWalker<'_>, ctx: &mut Cont
    let msg_foreign_keys = |action: ReferentialAction| {
        let allowed_actions = connector.referential_actions();

+        let additional_info = match (action, connector.provider_name()) {
+            (ReferentialAction::SetDefault, "mysql") => Some(formatdoc! {


No need to change it now, but for future PRs: we try hard to avoid connector-specific code in the core. This should be in the MySQL connector.

jkomyno · 2022-11-07T13:29:41Z

As discussed on Zoom with @janpio, this PR is no longer needed. We'll resort to warnings rather than errors in order to avoid scenarios where users that do not control their database schema (which may incorrectly have SET DEFAULT actions on MySQL) cannot upgrade to newer Prisma versions.

feat(psl): remove 'SetDefault' support on MySQL

6677254

jkomyno added this to the 4.6.0 milestone Nov 4, 2022

jkomyno added 2 commits November 4, 2022 16:51

chore: exclude provider="mysql" SetDefault from tests

902aece

test: add introspection-engine test for mysql SetDefault

9947502

jkomyno marked this pull request as ready for review November 4, 2022 16:49

jkomyno requested a review from a team as a code owner November 4, 2022 16:49

chore: minor formatting thing

20ff21f

janpio reviewed Nov 4, 2022

View reviewed changes

jkomyno added 2 commits November 4, 2022 18:04

chore: the 'introspect_set_default_should_fail' test is only run on m…

3b200f2

…ysql:8, added comment

chore: add quotes in comment

0f0f285

janpio reviewed Nov 4, 2022

View reviewed changes

tomhoule reviewed Nov 7, 2022

View reviewed changes

psl/psl/tests/attributes/relations/referential_actions.rs Outdated Show resolved Hide resolved

chore(psl): moved test to validation_tests suite

8db6678

tomhoule reviewed Nov 7, 2022

View reviewed changes

jkomyno mentioned this pull request Nov 7, 2022

Add /d/mysql-set-default shortcut to explain why the SetDefault referential action is no longer supported on mysql prisma/pris.ly#70

Closed

jkomyno closed this Nov 7, 2022

jkomyno removed this from the 4.6.0 milestone Nov 7, 2022

jkomyno mentioned this pull request Nov 14, 2022

Add SetDefault validation error with warnings when provider = "mysql" and relationMode = "foreignKeys" | default prisma/prisma#16259

Closed

janpio deleted the feat/validate-against-mysql-set-default branch November 15, 2023 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(psl): remove 'SetDefault' support on MySQL #3363

feat(psl): remove 'SetDefault' support on MySQL #3363

jkomyno commented Nov 4, 2022 •

edited

janpio Nov 4, 2022

jkomyno Nov 4, 2022

jkomyno Nov 4, 2022

janpio Nov 4, 2022

tomhoule Nov 7, 2022

jkomyno Nov 7, 2022

tomhoule Nov 7, 2022

janpio Nov 7, 2022

janpio Nov 7, 2022

janpio Nov 7, 2022 •

edited

janpio Nov 4, 2022

jkomyno Nov 4, 2022

janpio Nov 4, 2022

tomhoule Nov 7, 2022 •

edited

tomhoule Nov 7, 2022

jkomyno commented Nov 7, 2022

feat(psl): remove 'SetDefault' support on MySQL #3363

feat(psl): remove 'SetDefault' support on MySQL #3363

Conversation

jkomyno commented Nov 4, 2022 • edited

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

janpio Nov 7, 2022 • edited

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

tomhoule Nov 7, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkomyno commented Nov 7, 2022

jkomyno commented Nov 4, 2022 •

edited

janpio Nov 7, 2022 •

edited

tomhoule Nov 7, 2022 •

edited