Skip to content

Commit

Permalink
feat(psl): add validation + warning on missing indexes for `relationM…
Browse files Browse the repository at this point in the history
…ode = "prisma"` (#3429)

* feat(psl): add "push_warning" helper to Context

* test(psl): add validation tests for missing relation index warning

* feat(psl): add skeleton for missing relation indexes when relationMode="prisma"

* feat(psl): integrated "validate_missing_relation_indexes" into "validate" function

* feat(psl): extracted sets of indexes from the model containing a @relation

* feat(psl): fixed missing relation index detection

* feat(psl): add pretty_print support to warnings, defaulting to yellow colors

* feat(psl): add warning support to validation_tests suite

* chore(psl): fix clippy

* chore(psl): removed leftover code

* chore(psl): address review comments

* chore(psl): address review comments

* chore: removed redundant static declaration

* chore: fix clippy

* chore: removed noise

* chore: fix clippy, add one more unit test

* feat(psl): moved connector-specific logic to the "Connector" trait

* feat(psl): attempt to reduce allocations in "validate_missing_relation_indexes"

* chore(psl): applied review comments, fixed regression in "validate_missing_relation_indexes"

* fix: changed duplicated test

* feat(psl): add + Clone to "referencing_fields"

* chore: swapped at_unique tests

* chore: apply review comments on "validate_missing_relation_indexes" for better readability

* chore: removed model argument from "validate_missing_relation_indexes"

* chore: moved the "validate_missing_relation_indexes" function before the rest of index validations

* chore: changed pris.ly link in the "new_missing_index_on_emulated_relation" warning

* chore: updated "new_missing_index_on_emulated_relation" warning message

* chore: update Cargo.lock

* chore: removed deprecated "referentialIntegrity" preview feature in newly added tests

* chore(psl): updated warnings snapshot

* chore(psl): fixed unit tests?

* chore: apply review suggestions

* chore: rephase wording in warning
  • Loading branch information
jkomyno committed Nov 24, 2022
1 parent 8b710f7 commit c6a19ec
Show file tree
Hide file tree
Showing 18 changed files with 330 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions psl/builtin-connectors/src/mongodb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,9 @@ impl Connector for MongoDbDatamodelConnector {
fn allowed_relation_mode_settings(&self) -> enumflags2::BitFlags<RelationMode> {
RelationMode::Prisma.into()
}

/// Avoid checking whether the fields appearing in a `@relation` attribute are included in an index.
fn should_suggest_missing_referencing_fields_indexes(&self) -> bool {
false
}
}
1 change: 1 addition & 0 deletions psl/diagnostics/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ edition = "2021"
[dependencies]
colored = "2"
pest = "2.1.3"
indoc = "1"
13 changes: 11 additions & 2 deletions psl/diagnostics/src/warning.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use colored::{ColoredString, Colorize};

use crate::{
pretty_print::{pretty_print, DiagnosticColorer},
Span,
};
use colored::{ColoredString, Colorize};
use indoc::indoc;

/// A non-fatal warning emitted by the schema parser.
/// For fancy printing, please use the `pretty_print_error` function.
Expand All @@ -25,6 +25,15 @@ impl DatamodelWarning {
Self::new(message, span)
}

pub fn new_missing_index_on_emulated_relation(span: Span) -> DatamodelWarning {
let message = indoc! {
r#"With `relationMode = \"prisma\"`, no foreign keys are used, so relation fields will not benefit from the index usually created by the relational database under the hood.
This can lead to poor performance when querying these fields. We recommend adding an index manually.
Learn more at https://pris.ly/d/relation-mode#indexes"#
};
Self::new(message.to_owned(), span)
}

/// The user-facing warning message.
pub fn message(&self) -> &str {
&self.message
Expand Down
4 changes: 2 additions & 2 deletions psl/parser-database/src/walkers/relation_field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,12 @@ impl<'db> RelationFieldWalker<'db> {
}

/// The fields in the `fields: [...]` argument in the forward relation field.
pub fn referencing_fields(self) -> Option<impl ExactSizeIterator<Item = ScalarFieldWalker<'db>>> {
pub fn referencing_fields(self) -> Option<impl ExactSizeIterator<Item = ScalarFieldWalker<'db>> + Clone> {
self.fields()
}

/// The fields in the `fields: [...]` argument in the forward relation field.
pub fn fields(self) -> Option<impl ExactSizeIterator<Item = ScalarFieldWalker<'db>>> {
pub fn fields(self) -> Option<impl ExactSizeIterator<Item = ScalarFieldWalker<'db>> + Clone> {
let model_id = self.model_id;
let attributes = self.attributes();
attributes.fields.as_ref().map(move |fields| {
Expand Down
6 changes: 6 additions & 0 deletions psl/psl-core/src/datamodel_connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,12 @@ pub trait Connector: Send + Sync {
self.has_capability(ConnectorCapability::RelationFieldsInArbitraryOrder)
}

/// If true, the schema validator function checks whether the referencing fields in a `@relation` attribute
/// are included in an index.
fn should_suggest_missing_referencing_fields_indexes(&self) -> bool {
true
}

fn native_type_to_string(&self, instance: &NativeTypeInstance) -> String {
let (name, args) = self.native_type_to_parts(instance);
let args = if args.is_empty() {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ pub(super) fn validate(ctx: &mut Context<'_>) {
relation_fields::ignored_related_model(field, ctx);
relation_fields::referential_actions(field, ctx);
relation_fields::map(field, ctx);
relation_fields::validate_missing_relation_indexes(field, ctx);
}

for index in model.indexes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
diagnostics::DatamodelError,
validate::validation_pipeline::context::Context,
};
use diagnostics::DatamodelWarning;
use enumflags2::BitFlags;
use itertools::Itertools;
use parser_database::{
Expand Down Expand Up @@ -249,3 +250,67 @@ pub(super) fn map(field: RelationFieldWalker<'_>, ctx: &mut Context<'_>) {
validate_db_name(field.model().name(), relation_attr, field.mapped_name(), ctx, false);
}
}

pub(super) fn validate_missing_relation_indexes(relation_field: RelationFieldWalker<'_>, ctx: &mut Context<'_>) {
if !ctx.connector.should_suggest_missing_referencing_fields_indexes() || ctx.relation_mode != RelationMode::Prisma {
return;
}

if let Some(fields) = relation_field.referencing_fields() {
let model = relation_field.model();
// Considers all fields that should be part of an index in the given model, w.r.t. to left-wise inclusion.
let referencing_fields_it = fields.map(|field| field.field_id());

// Considers all groups of indexes explicitly declared in the given model.
// An index group can be:
// - a singleton (@unique or @id)
// - an ordered set (@@unique or @@index)
let index_field_groups = model.indexes();

let referencing_fields_appear_in_index = index_field_groups
.map(|index_walker| index_walker.fields().map(|index| index.field_id()))
.any(|index_fields_it| {
let fields_it = referencing_fields_it.clone();
is_leftwise_included_it(fields_it, index_fields_it)
});

if !referencing_fields_appear_in_index {
let ast_field = relation_field.ast_field();
let span = ast_field
.span_for_attribute("relation")
.unwrap_or_else(|| ast_field.span());
ctx.push_warning(DatamodelWarning::new_missing_index_on_emulated_relation(span));
}
}
}

/// An subgroup is left-wise included in a supergroup if the subgroup is contained in the supergroup, and all the entries of
/// the left-most entries of the supergroup match the order of definitions of the subgroup.
/// More formally: { x_1, x_2, ..., x_n } is left-wise included in { y_1, y_2, ..., y_m } if and only if
/// n <= m and x_i = y_i for all i in [1, n].
fn is_leftwise_included_it<T>(subgrop: impl ExactSizeIterator<Item = T>, supergroup: impl Iterator<Item = T>) -> bool
where
T: PartialEq,
{
supergroup.take(subgrop.len()).eq(subgrop)
}

#[cfg(test)]
mod tests {
use super::is_leftwise_included_it;
#[test]
fn test_is_left_wise_included() {
let item = vec![1, 2];
let group = vec![1, 2, 3, 4];
assert_eq!(is_leftwise_included_it(item.iter(), group.iter()), true);
let item = vec![1, 2, 3, 4];
let group = vec![1, 2, 3, 4];
assert_eq!(is_leftwise_included_it(item.iter(), group.iter()), true);
let item = vec![1, 2, 3, 4];
let group = vec![1, 2];
assert_eq!(is_leftwise_included_it(item.iter(), group.iter()), false);
let item = vec![2, 3];
let group = vec![1, 2, 3, 4];
assert_eq!(is_leftwise_included_it(item.iter(), group.iter()), 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,17 @@
// no missing relation index validation on relationMode = "foreignKeys" / no relationMode at all.

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,20 @@
// no relation index validation warning on relationMode = "prisma" when a referenced field is already in @@unique.

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])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// no relation index validation warning on relationMode = "prisma" when a referenced field is already in @unique.

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])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// add missing relation index validation warning on relationMode = "prisma".

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])
}
// warning: With `relationMode = \"prisma\"`, no foreign keys are used, so relation fields will not benefit from the index usually created by the relational database under the hood.
// This can lead to poor performance when querying these fields. We recommend adding an index manually.
// Learn more at https://pris.ly/d/relation-mode#indexes
// --> schema.prisma:17
//  | 
// 16 |  userId Int
// 17 |  user SomeUser @relation(fields: [userId], references: [id])
//  | 
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// add missing relation index validation warning on relationMode = "prisma".

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])
}
// warning: With `relationMode = \"prisma\"`, no foreign keys are used, so relation fields will not benefit from the index usually created by the relational database under the hood.
// This can lead to poor performance when querying these fields. We recommend adding an index manually.
// Learn more at https://pris.ly/d/relation-mode#indexes
// --> schema.prisma:23
//  | 
// 22 |  userIdC Int
// 23 |  user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])
//  | 
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// add missing relation index validation warning on relationMode = "prisma".

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])
}
// warning: With `relationMode = \"prisma\"`, no foreign keys are used, so relation fields will not benefit from the index usually created by the relational database under the hood.
// This can lead to poor performance when querying these fields. We recommend adding an index manually.
// Learn more at https://pris.ly/d/relation-mode#indexes
// --> schema.prisma:23
//  | 
// 22 |  userIdC Int
// 23 |  user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])
//  | 
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// add missing relation index validation warning on relationMode = "prisma".

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])
}
// warning: With `relationMode = \"prisma\"`, no foreign keys are used, so relation fields will not benefit from the index usually created by the relational database under the hood.
// This can lead to poor performance when querying these fields. We recommend adding an index manually.
// Learn more at https://pris.ly/d/relation-mode#indexes
// --> schema.prisma:23
//  | 
// 22 |  userIdC Int @unique
// 23 |  user SomeUser @relation(fields: [userIdA, userIdB, userIdC], references: [idA, idB, idC])
//  | 

0 comments on commit c6a19ec

Please sign in to comment.