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(psl): add validation + warning on missing indexes for relationMode = "prisma" #3429

Merged
merged 36 commits into from
Nov 24, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
01f1f2c
feat(psl): add "push_warning" helper to Context
jkomyno Nov 22, 2022
32b45c0
test(psl): add validation tests for missing relation index warning
jkomyno Nov 22, 2022
a188095
feat(psl): add skeleton for missing relation indexes when relationMod…
jkomyno Nov 22, 2022
97ccc4d
feat(psl): integrated "validate_missing_relation_indexes" into "valid…
jkomyno Nov 22, 2022
2d40109
feat(psl): extracted sets of indexes from the model containing a @rel…
jkomyno Nov 22, 2022
33e0912
feat(psl): fixed missing relation index detection
jkomyno Nov 22, 2022
f391f4b
feat(psl): add pretty_print support to warnings, defaulting to yellow…
jkomyno Nov 22, 2022
109d558
feat(psl): add warning support to validation_tests suite
jkomyno Nov 22, 2022
84e891f
chore(psl): fix clippy
jkomyno Nov 22, 2022
5701214
chore(psl): removed leftover code
jkomyno Nov 22, 2022
69c20c7
chore(psl): address review comments
jkomyno Nov 22, 2022
a312eb4
chore(psl): address review comments
jkomyno Nov 22, 2022
a78867e
chore: removed redundant static declaration
jkomyno Nov 22, 2022
2d25679
Merge branch 'feat/psl-support-warnings-in-tests' into feat/add-index…
jkomyno Nov 22, 2022
34e1b44
chore: fix clippy
jkomyno Nov 22, 2022
127c11b
chore: removed noise
jkomyno Nov 22, 2022
3818edf
Merge branch 'main' into feat/add-index-warnings
jkomyno Nov 22, 2022
f1ddc0e
chore: fix clippy, add one more unit test
jkomyno Nov 22, 2022
581b5ce
feat(psl): moved connector-specific logic to the "Connector" trait
jkomyno Nov 23, 2022
ecaf05e
feat(psl): attempt to reduce allocations in "validate_missing_relatio…
jkomyno Nov 23, 2022
a5a94d3
chore(psl): applied review comments, fixed regression in "validate_mi…
jkomyno Nov 23, 2022
b747f6b
fix: changed duplicated test
jkomyno Nov 23, 2022
afc42aa
feat(psl): add + Clone to "referencing_fields"
jkomyno Nov 23, 2022
81267a3
chore: swapped at_unique tests
jkomyno Nov 23, 2022
6302434
chore: apply review comments on "validate_missing_relation_indexes" f…
jkomyno Nov 23, 2022
cf302ba
chore: removed model argument from "validate_missing_relation_indexes"
jkomyno Nov 23, 2022
eb283a1
chore: moved the "validate_missing_relation_indexes" function before …
jkomyno Nov 23, 2022
98526a7
chore: changed pris.ly link in the "new_missing_index_on_emulated_rel…
jkomyno Nov 23, 2022
48312a4
chore: updated "new_missing_index_on_emulated_relation" warning message
jkomyno Nov 24, 2022
c57c297
chore: update Cargo.lock
jkomyno Nov 24, 2022
0832a54
chore: removed deprecated "referentialIntegrity" preview feature in n…
jkomyno Nov 24, 2022
873e328
chore(psl): updated warnings snapshot
jkomyno Nov 24, 2022
4233793
Merge branch 'main' into feat/add-index-warnings
jkomyno Nov 24, 2022
258b019
chore(psl): fixed unit tests?
jkomyno Nov 24, 2022
d050fcd
chore: apply review suggestions
jkomyno Nov 24, 2022
52a204e
chore: rephase wording in warning
jkomyno Nov 24, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions psl/diagnostics/src/warning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ impl DatamodelWarning {
Self::new(message, span)
}

pub fn new_missing_index_on_emulated_relation(span: Span) -> DatamodelWarning {
let message = format!("The fields referenced in a `@relation` should be indexed to prevent potential performance penalties. Learn more at https://pris.ly/d/relation-mode-prisma-indexes.");
Self::new(message, span)
janpio marked this conversation as resolved.
Show resolved Hide resolved
}

/// The user-facing warning message.
pub fn message(&self) -> &str {
&self.message
Expand Down
7 changes: 6 additions & 1 deletion psl/psl-core/src/validate/validation_pipeline/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::{
datamodel_connector::{Connector, RelationMode},
Datasource, PreviewFeature,
};
use diagnostics::{DatamodelError, Diagnostics};
use diagnostics::{DatamodelError, DatamodelWarning, Diagnostics};
use enumflags2::BitFlags;

/// The validation context. The lifetime parameter is _not_ the AST lifetime, but the subtype of
Expand All @@ -24,4 +24,9 @@ impl Context<'_> {
pub(super) fn push_error(&mut self, error: DatamodelError) {
self.diagnostics.push_error(error);
}

/// Pure convenience method. Forwards to Diagnostics::push_warning().
pub(super) fn push_warning(&mut self, warning: DatamodelWarning) {
self.diagnostics.push_warning(warning);
}
}
Comment on lines +28 to 32
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ mod indexes;
mod models;
mod names;
mod relation_fields;
mod relation_indexes;
mod relations;

use super::context::Context;
Expand Down Expand Up @@ -124,6 +125,10 @@ pub(super) fn validate(ctx: &mut Context<'_>) {
fields::validate_length_used_with_correct_types(field_attribute, attribute, ctx);
}
}

for field in model.relation_fields() {
relation_indexes::validate_missing_relation_indexes(model, field, ctx);
}
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
}

if ctx.connector.supports_enums() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use diagnostics::DatamodelWarning;
use parser_database::{
ast::ModelId,
walkers::{RelationFieldWalker, Walker},
};

use crate::{datamodel_connector::RelationMode, validate::validation_pipeline::context::Context};

// { x_1, x_2, ..., x_n } is left-wise included in { y_1, y_2, ..., y_m } if and only if
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
// n <= m and x_i = y_i for all i in [1, n].
fn is_left_wise_included<T>(item: &Vec<T>, group: &Vec<T>) -> bool
where
T: PartialEq,
{
group.iter().take(item.len()).eq(item.iter())
}

pub(super) fn validate_missing_relation_indexes(
_model: Walker<'_, ModelId>,
relation_field: RelationFieldWalker<'_>,
ctx: &mut Context<'_>,
) {
dbg!("Inspecting: {:?}", relation_field.name());
jkomyno marked this conversation as resolved.
Show resolved Hide resolved

let is_provider_mongodb = ctx
.datasource
.map(|datasource| datasource.active_provider == "mongodb")
.unwrap_or(false);

if is_provider_mongodb || ctx.relation_mode != RelationMode::Prisma {
return;
}
tomhoule marked this conversation as resolved.
Show resolved Hide resolved

if let Some(fields) = relation_field.referenced_fields() {
// vector of ids of the fields that should be part of an index in the given model, w.r.t. to left-wise inclusion.
let relation_fields = fields.map(|field| field.field_id()).collect::<Vec<_>>();

// for index_walker in model.indexes() {
// dbg!("exploring index: {:?}", index_walker.name());
// index_walker.fields().for_each(|index| {
// dbg!(" index: {:?}", index);
// });
// }

// vector of all @unique/@@unique attributes in the given model
// TODO: figure out how to retrieve these attributes
let unique_sets: Vec<Vec<_>> = vec![];

// vector of all @@index attributes in the given model
// TODO: figure out how to retrieve these attributes
let index_sets: Vec<Vec<_>> = vec![];
jkomyno marked this conversation as resolved.
Show resolved Hide resolved

let relation_fields_appear_in_unique = unique_sets
.iter()
.any(|unique_set| is_left_wise_included(&relation_fields, unique_set));

let relation_fields_appear_in_index = index_sets
.iter()
.any(|index_set| is_left_wise_included(&relation_fields, index_set));
jkomyno marked this conversation as resolved.
Show resolved Hide resolved

if !(relation_fields_appear_in_unique || relation_fields_appear_in_index) {
let span = relation_field.ast_field().field_type.span();
Copy link
Contributor

Choose a reason for hiding this comment

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

The highlighting is going to be on the field type, so only the OtherModel part in other OtherModel @relation(...). I think it would be more readable for users if we highlighted the whole field. So relation_field.ast_field().span.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I had a doubt about this span here. I think we mentioned elsewhere in the Notion doc that we wanted to highlight the @relation attribute.
Hence, I'd propose the following

let span = ast_field
  .span_for_attribute("relation")
  .unwrap_or_else(|| ast_field.span());

which falls back to your solution

ctx.push_warning(DatamodelWarning::new_missing_index_on_emulated_relation(span));
}
}
}

#[cfg(test)]
mod tests {
use super::is_left_wise_included;

#[test]
fn test_is_left_wise_included() {
let item = vec![1, 2];
let group = vec![1, 2, 3, 4];
assert_eq!(is_left_wise_included(&item, &group), true);

let item = vec![1, 2, 3, 4];
let group = vec![1, 2];
assert_eq!(is_left_wise_included(&item, &group), false);

let item = vec![2, 3];
let group = vec![1, 2, 3, 4];
assert_eq!(is_left_wise_included(&item, &group), false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// no missing relation index validation on mongodb.

datasource db {
provider = "mongodb"
url = env("TEST_DATABASE_URL")
}

model SomeUser {
id String @id @map("_id") @default(auto()) @db.ObjectId
posts Post[]
}

model Post {
id String @id @map("_id") @default(auto()) @db.ObjectId
userId String
user SomeUser @relation(fields: [userId], references: [id])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// no missing relation index validation on relationMode = "foreignKeys".

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

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

model SomeUser {
id Int @id
posts Post[]
}

model Post {
id Int @id
userId Int
user SomeUser @relation(fields: [userId], references: [id])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// no relation index validation warning on relationMode = "prisma" when a referenced field is already in @@unique.

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
id Int @id
profile Profile?
}

model Profile {
id Int @id
userId Int
user SomeUser? @relation(fields: [userId], references: [id])

@@unique([userId])
}
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// no relation index validation warning on relationMode = "prisma" when a referenced field is already @unique.

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
id Int @id
profile Profile?
}

model Profile {
id Int @id
userId Int @unique
user SomeUser? @relation(fields: [userId], references: [id])
}
jkomyno marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// add missing relation index validation warning on relationMode = "prisma".

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
id Int @id
posts Post[]
}

model Post {
id Int @id
userId Int
user SomeUser @relation(fields: [userId], references: [id])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// add missing relation index validation warning on relationMode = "prisma".

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
idA Int
idB Int
idC Int
posts Post[]

@@id([idA, idB, idC])
}

model Post {
id Int @id
userIdA Int
userIdB Int
userIdC Int
user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])

@@index([userIdA, userIdB])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// add missing relation index validation warning on relationMode = "prisma".

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
idA Int
idB Int
idC Int
posts Post[]

@@id([idA, idB, idC])
}

model Post {
id Int @id
userIdA Int
userIdB Int
userIdC Int
user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])

@@index([userIdB, userIdC])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// add missing relation index validation warning on relationMode = "prisma".

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
idA Int
idB Int
idC Int
posts Post[]

@@id([idA, idB, idC])
}

model Post {
id Int @id
userIdA Int @unique
userIdB Int
userIdC Int @unique
user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])

@@unique([userIdA, userIdB])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// add missing relation index validation warning on relationMode = "prisma".

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

datasource db {
provider = "mysql"
url = env("TEST_DATABASE_URL")
relationMode = "prisma"
}

model SomeUser {
idA Int
idB Int
idC Int
posts Post[]

@@id([idA, idB, idC])
}

model Post {
id Int @id
userIdA Int
userIdB Int
userIdC Int
user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])
}