Skip to content

Commit

Permalink
feat(psl): add warning when SetDefault is used on mysql with `rel…
Browse files Browse the repository at this point in the history
…ationMode = "foreignKeys"` (#3435)

* feat(psl): support connector-specific warnings in DatamodelWarning

* feat(psl): add warning when SetDefault is used in MySQL with foreignKeys mode active

* test(psl): add tests for SetDefault warning on MySQL

* test: add introspection and migration tests for SetDefault warning on MySQL

* chore: removed deprecated previewFeature referentialIntegrity

* chore: fix snapshots in unit tests

* test: exclude Vitess from "introspect_set_default_should_warn" test

* chore: simplify conditional for SetDefault
  • Loading branch information
jkomyno committed Nov 24, 2022
1 parent c6a19ec commit b56b159
Show file tree
Hide file tree
Showing 16 changed files with 251 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod cockroachdb;
mod mysql;
mod sqlite;

use barrel::types;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
use introspection_engine_tests::test_api::*;

// Older versions of MySQL (5.6+) raise a syntax error on `CREATE TABLE` declarations with `SET DEFAULT` referential actions,
// so we can skip introspecting those. MariaDb 10.0 suffers from the same issue.
// We should see validation warnings on MySQL 8+.
#[test_connector(tags(Mysql8), exclude(Vitess))]
async fn introspect_set_default_should_warn(api: &TestApi) -> TestResult {
let setup = r#"
CREATE TABLE `SomeUser` (
`id` INTEGER NOT NULL AUTO_INCREMENT,
PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
CREATE TABLE `Post` (
`id` INTEGER NOT NULL AUTO_INCREMENT,
`userId` INTEGER NULL DEFAULT 3,
PRIMARY KEY (`id`)
) DEFAULT CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;
ALTER TABLE `Post` ADD CONSTRAINT `Post_userId_fkey`
FOREIGN KEY (`userId`) REFERENCES `SomeUser`(`id`)
ON DELETE SET DEFAULT ON UPDATE SET DEFAULT;
"#;

api.raw_cmd(setup).await;

let expected_schema = expect![[r#"
generator client {
provider = "prisma-client-js"
}
datasource db {
provider = "mysql"
url = "env(TEST_DATABASE_URL)"
}
model Post {
id Int @id @default(autoincrement())
userId Int? @default(3)
SomeUser SomeUser? @relation(fields: [userId], references: [id], onDelete: SetDefault, onUpdate: SetDefault)
@@index([userId], map: "Post_userId_fkey")
}
model SomeUser {
id Int @id @default(autoincrement())
Post Post[]
}
"#]];

expected_schema.assert_eq(&api.introspect().await?);
let schema = psl::parse_schema(expected_schema.data())?;

let warning_messages = schema
.diagnostics
.warnings_to_pretty_string("schema.prisma", &schema.db.source());

let expected_validation = expect![[r#"
warning: Using SetDefault on MySQL may yield to unexpected results, as the database will silently change the referential action to `NoAction`.
--> schema.prisma:14
 | 
13 |  userId Int? @default(3)
14 |  SomeUser SomeUser? @relation(fields: [userId], references: [id], onDelete: SetDefault, onUpdate: SetDefault)
 | 
warning: Using SetDefault on MySQL may yield to unexpected results, as the database will silently change the referential action to `NoAction`.
--> schema.prisma:14
 | 
13 |  userId Int? @default(3)
14 |  SomeUser SomeUser? @relation(fields: [userId], references: [id], onDelete: SetDefault, onUpdate: SetDefault)
 | 
"#]];
expected_validation.assert_eq(&warning_messages);

Ok(())
}
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,8 @@ fn on_delete_referential_actions_should_work(api: TestApi) {
}

// 5.6 and 5.7 doesn't let you `SET DEFAULT` without setting the default value
// (even if nullable). Maria will silently just use `RESTRICT` instead.
// (even if nullable). MySQL 8.0+ & MariaDB 10.0 allow you to create a table with
// `SET DEFAULT`, but will silently use `NO ACTION` / `RESTRICT` instead.
#[test_connector(exclude(Mysql56, Mysql57, Mariadb, Mssql, Vitess, CockroachDb))]
fn on_delete_set_default_should_work(api: TestApi) {
let dm = r#"
Expand Down Expand Up @@ -661,7 +662,8 @@ fn on_update_referential_actions_should_work(api: TestApi) {
}

// 5.6 and 5.7 doesn't let you `SET DEFAULT` without setting the default value
// (even if nullable). Maria will silently just use `RESTRICT` instead.
// (even if nullable). MySQL 8.0+ & MariaDB 10.0 allow you to create a table with
// `SET DEFAULT`, but will silently use `NO ACTION` / `RESTRICT` instead.
#[test_connector(exclude(Mysql56, Mysql57, Mariadb, Mssql, Vitess, CockroachDb))]
fn on_update_set_default_should_work(api: TestApi) {
let dm = r#"
Expand Down
5 changes: 3 additions & 2 deletions psl/builtin-connectors/src/cockroach_datamodel_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use enumflags2::BitFlags;
use lsp_types::{CompletionItem, CompletionItemKind, CompletionList};
use psl_core::{
datamodel_connector::{
Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance, StringFilter,
Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance, RelationMode,
StringFilter,
},
diagnostics::{DatamodelError, Diagnostics},
parser_database::{
Expand Down Expand Up @@ -192,7 +193,7 @@ impl Connector for CockroachDatamodelConnector {
}
}

fn validate_model(&self, model: ModelWalker<'_>, diagnostics: &mut Diagnostics) {
fn validate_model(&self, model: ModelWalker<'_>, _: RelationMode, diagnostics: &mut Diagnostics) {
validations::autoincrement_validations(model, diagnostics);

for index in model.indexes() {
Expand Down
2 changes: 1 addition & 1 deletion psl/builtin-connectors/src/mongodb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Connector for MongoDbDatamodelConnector {
BitFlags::empty()
}

fn validate_model(&self, model: ModelWalker<'_>, errors: &mut Diagnostics) {
fn validate_model(&self, model: ModelWalker<'_>, _: RelationMode, errors: &mut Diagnostics) {
validations::id_must_be_defined(model, errors);

if let Some(pk) = model.primary_key() {
Expand Down
11 changes: 9 additions & 2 deletions psl/builtin-connectors/src/mssql_datamodel_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use connection_string::JdbcString;
use enumflags2::BitFlags;
use lsp_types::{CompletionItem, CompletionItemKind, CompletionList};
use psl_core::{
datamodel_connector::{Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance},
datamodel_connector::{
Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance, RelationMode,
},
diagnostics::{Diagnostics, Span},
parser_database::{self, ast, ParserDatabase, ReferentialAction, ScalarType},
};
Expand Down Expand Up @@ -228,7 +230,12 @@ impl Connector for MsSqlDatamodelConnector {
}
}

fn validate_model(&self, model: parser_database::walkers::ModelWalker<'_>, errors: &mut Diagnostics) {
fn validate_model(
&self,
model: parser_database::walkers::ModelWalker<'_>,
_: RelationMode,
errors: &mut Diagnostics,
) {
for index in model.indexes() {
validations::index_uses_correct_field_types(self, index, errors);
}
Expand Down
12 changes: 10 additions & 2 deletions psl/builtin-connectors/src/mysql_datamodel_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ pub use native_types::MySqlType;

use enumflags2::BitFlags;
use psl_core::{
datamodel_connector::{Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance},
datamodel_connector::{
Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance, RelationMode,
},
diagnostics::{DatamodelError, Diagnostics, Span},
parser_database::{walkers, ReferentialAction, ScalarType},
};
Expand Down Expand Up @@ -211,14 +213,20 @@ impl Connector for MySqlDatamodelConnector {
}
}

fn validate_model(&self, model: walkers::ModelWalker<'_>, errors: &mut Diagnostics) {
fn validate_model(&self, model: walkers::ModelWalker<'_>, relation_mode: RelationMode, errors: &mut Diagnostics) {
for index in model.indexes() {
validations::field_types_can_be_used_in_an_index(self, index, errors);
}

if let Some(pk) = model.primary_key() {
validations::field_types_can_be_used_in_a_primary_key(self, pk, errors);
}

if relation_mode.uses_foreign_keys() {
for field in model.relation_fields() {
validations::uses_native_referential_action_set_default(self, field, errors);
}
}
}

fn constraint_violation_scopes(&self) -> &'static [ConstraintScope] {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use psl_core::diagnostics::{DatamodelWarning, Span};
use psl_core::parser_database::ast::WithSpan;
use psl_core::parser_database::ReferentialAction;
use psl_core::{
datamodel_connector::{walker_ext_traits::ScalarFieldWalkerExt, Connector},
diagnostics::Diagnostics,
parser_database::walkers::{IndexWalker, PrimaryKeyWalker},
parser_database::walkers::{IndexWalker, PrimaryKeyWalker, RelationFieldWalker},
};

const LENGTH_GUIDE: &str = " Please use the `length` argument to the field in the index definition to allow this.";
Expand Down Expand Up @@ -89,3 +92,35 @@ pub(crate) fn field_types_can_be_used_in_a_primary_key(
break;
}
}

pub(crate) fn uses_native_referential_action_set_default(
connector: &dyn Connector,
field: RelationFieldWalker<'_>,
diagnostics: &mut Diagnostics,
) {
let get_span = |referential_action_type: &str| -> Span {
field
.ast_field()
.span_for_argument("relation", referential_action_type)
.unwrap_or_else(|| field.ast_field().span())
};

let warning_msg = || {
format!(
"Using {set_default} on {connector} may yield to unexpected results, as the database will silently change the referential action to `{no_action}`.",
set_default = ReferentialAction::SetDefault.as_str(),
connector = connector.name(),
no_action = ReferentialAction::NoAction.as_str(),
)
};

if let Some(ReferentialAction::SetDefault) = field.explicit_on_delete() {
let span = get_span("onDelete");
diagnostics.push_warning(DatamodelWarning::new(warning_msg(), span));
}

if let Some(ReferentialAction::SetDefault) = field.explicit_on_update() {
let span = get_span("onUpdate");
diagnostics.push_warning(DatamodelWarning::new(warning_msg(), span));
}
}
5 changes: 3 additions & 2 deletions psl/builtin-connectors/src/postgres_datamodel_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use enumflags2::BitFlags;
use lsp_types::{CompletionItem, CompletionItemKind, CompletionList, InsertTextFormat};
use psl_core::{
datamodel_connector::{
Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance, StringFilter,
Connector, ConnectorCapability, ConstraintScope, NativeTypeConstructor, NativeTypeInstance, RelationMode,
StringFilter,
},
diagnostics::Diagnostics,
parser_database::{ast, walkers, IndexAlgorithm, OperatorClass, ParserDatabase, ReferentialAction, ScalarType},
Expand Down Expand Up @@ -361,7 +362,7 @@ impl Connector for PostgresDatamodelConnector {
}
}

fn validate_model(&self, model: walkers::ModelWalker<'_>, errors: &mut Diagnostics) {
fn validate_model(&self, model: walkers::ModelWalker<'_>, _: RelationMode, errors: &mut Diagnostics) {
for index in model.indexes() {
validations::compatible_native_types(index, self, errors);
validations::generalized_index_validations(index, self, errors);
Expand Down
11 changes: 11 additions & 0 deletions psl/diagnostics/src/collection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ impl Diagnostics {

String::from_utf8_lossy(&message).into_owned()
}

pub fn warnings_to_pretty_string(&self, file_name: &str, datamodel_string: &str) -> String {
let mut message: Vec<u8> = Vec::new();

for warn in self.warnings() {
warn.pretty_print(&mut message, file_name, datamodel_string)
.expect("printing datamodel warning");
}

String::from_utf8_lossy(&message).into_owned()
}
}

impl From<DatamodelError> for Diagnostics {
Expand Down
4 changes: 3 additions & 1 deletion psl/diagnostics/src/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ pub struct DatamodelWarning {
}

impl DatamodelWarning {
fn new(message: String, span: Span) -> DatamodelWarning {
/// You should avoid using this constructor directly when possible, and define warnings as public methods of this class.
/// The constructor is only left public for supporting connector-specific warnings (which should not live in the core).
pub fn new(message: String, span: Span) -> DatamodelWarning {
DatamodelWarning { message, span }
}

Expand Down
2 changes: 1 addition & 1 deletion psl/psl-core/src/datamodel_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ pub trait Connector: Send + Sync {
}

fn validate_enum(&self, _enum: walkers::EnumWalker<'_>, _: &mut Diagnostics) {}
fn validate_model(&self, _model: walkers::ModelWalker<'_>, _: &mut Diagnostics) {}
fn validate_model(&self, _model: walkers::ModelWalker<'_>, _: RelationMode, _: &mut Diagnostics) {}
fn validate_datasource(&self, _: BitFlags<PreviewFeature>, _: &Datasource, _: &mut Diagnostics) {}

fn validate_scalar_field_unknown_default_functions(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub(crate) fn primary_key_connector_specific(model: ModelWalker<'_>, ctx: &mut C
}

pub(super) fn connector_specific(model: ModelWalker<'_>, ctx: &mut Context<'_>) {
ctx.connector.validate_model(model, ctx.diagnostics)
ctx.connector.validate_model(model, ctx.relation_mode, ctx.diagnostics)
}

pub(super) fn id_has_fields(model: ModelWalker<'_>, ctx: &mut Context<'_>) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
generator client {
provider = "prisma-client-js"
}

datasource db {
provider = "mysql"
url = "mysql://"
relationMode = "prisma"
}

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

model B {
id Int @id
aId Int? @default(3)
a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
}
// error: Error validating: Invalid referential action: `SetDefault`. Allowed values: (`Cascade`, `Restrict`, `NoAction`, `SetNull`)
// --> schema.prisma:19
//  | 
// 18 |  aId Int? @default(3)
// 19 |  a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
//  | 
// error: Error validating: Invalid referential action: `SetDefault`. Allowed values: (`Cascade`, `Restrict`, `NoAction`, `SetNull`)
// --> schema.prisma:19
//  | 
// 18 |  aId Int? @default(3)
// 19 |  a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
//  | 
27 changes: 27 additions & 0 deletions psl/psl/tests/validation/mysql/set_default_warning.prisma
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
datasource db {
provider = "mysql"
url = "mysql://"
}

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

model B {
id Int @id
aId Int? @default(3)
a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
}
// warning: Using SetDefault on MySQL may yield to unexpected results, as the database will silently change the referential action to `NoAction`.
// --> schema.prisma:14
//  | 
// 13 |  aId Int? @default(3)
// 14 |  a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
//  | 
// warning: Using SetDefault on MySQL may yield to unexpected results, as the database will silently change the referential action to `NoAction`.
// --> schema.prisma:14
//  | 
// 13 |  aId Int? @default(3)
// 14 |  a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
//  | 
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
generator client {
provider = "prisma-client-js"
}

datasource db {
provider = "mysql"
url = "mysql://"
relationMode = "foreignKeys"
}

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

model B {
id Int @id
aId Int? @default(3)
a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
}
// warning: Using SetDefault on MySQL may yield to unexpected results, as the database will silently change the referential action to `NoAction`.
// --> schema.prisma:19
//  | 
// 18 |  aId Int? @default(3)
// 19 |  a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
//  | 
// warning: Using SetDefault on MySQL may yield to unexpected results, as the database will silently change the referential action to `NoAction`.
// --> schema.prisma:19
//  | 
// 18 |  aId Int? @default(3)
// 19 |  a A? @relation(fields: [aId], references: [id], onUpdate: SetDefault, onDelete: SetDefault)
//  | 

0 comments on commit b56b159

Please sign in to comment.