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

fix(psl): composite type validation in indices #4401

Merged
merged 9 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub(super) fn validate(ctx: &mut Context<'_>) {
indexes::supports_clustering_setting(index, ctx);
indexes::clustering_can_be_defined_only_once(index, ctx);
indexes::opclasses_are_not_allowed_with_other_than_normal_indices(index, ctx);
indexes::composite_types_are_not_allowed_in_index(index, ctx);
indexes::composite_type_in_compound_unique_index(index, ctx);
Copy link
Contributor

Choose a reason for hiding this comment

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

What about normal indexes? Are those supported with a mix of scalar and composite fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They don't actually hit this error (fulltext also doesn't). Not to say they're supported but they run into different issues that are unrelated to the original crash report. Only @@unique causes a panic from what I've found


for field_attribute in index.scalar_field_attributes() {
let span = index.ast_attribute().span;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,20 +386,25 @@ pub(crate) fn opclasses_are_not_allowed_with_other_than_normal_indices(index: In
}
}

pub(crate) fn composite_types_are_not_allowed_in_index(index: IndexWalker<'_>, ctx: &mut Context<'_>) {
for field in index.fields() {
if field.scalar_field_type().as_composite_type().is_some() {
let message = format!(
"Indexes can only contain scalar attributes. Please remove {:?} from the argument list of the indexes.",
field.name()
);
ctx.push_error(DatamodelError::new_attribute_validation_error(
&message,
index.attribute_name(),
index.ast_attribute().span,
));
return;
}
pub(crate) fn composite_type_in_compound_unique_index(index: IndexWalker<'_>, ctx: &mut Context<'_>) {
if !index.is_unique() {
return;
}

let composite_type = index
.fields()
.find(|f| f.scalar_field_type().as_composite_type().is_some());

if index.fields().len() > 1 && composite_type.is_some() {
let message = format!(
"Prisma does not currently support composite types in compound unique indices, please remove {:?} from the index. See https://pris.ly/d/mongodb-composite-compound-indices for more details",
composite_type.unwrap().name()
);
ctx.push_error(DatamodelError::new_attribute_validation_error(
&message,
index.attribute_name(),
index.ast_attribute().span,
));
Druue marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
2 changes: 1 addition & 1 deletion query-engine/prisma-models/src/field/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ impl ScalarField {

match scalar_field_type {
ScalarFieldType::CompositeType(_) => {
unreachable!("Cannot convert a composite type to a type identifier. This error is typically caused by mistakenly using a composite type within a composite index.",)
unreachable!("This shouldn't be reached; composite types are not supported in compound unique indices.",)
Copy link
Contributor

Choose a reason for hiding this comment

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

Less cryptic message 👍🏾

}
ScalarFieldType::Enum(x) => TypeIdentifier::Enum(x),
ScalarFieldType::BuiltInScalar(scalar) => scalar.into(),
Expand Down
160 changes: 144 additions & 16 deletions query-engine/prisma-models/tests/datamodel_converter_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,31 +38,159 @@ fn converting_enums() {
}
}

// region: composite
#[test]
fn converting_composite_types() {
fn converting_composite_types_compound() {
let res = psl::parse_schema(
r#"
datasource db {
provider = "mongodb"
url = "mongodb://localhost:27017/hello"
}
datasource db {
provider = "mongodb"
url = "mongodb://localhost:27017/hello"
}

model MyModel {
id String @id @default(auto()) @map("_id") @db.ObjectId
attribute Attribute
model Post {
id String @id @default(auto()) @map("_id") @db.ObjectId
author User @relation(fields: [authorId], references: [id])
authorId String @db.ObjectId
attributes Attribute[]

@@index([authorId, attributes])
}

type Attribute {
name String
value String
group String
}

model User {
id String @id @default(auto()) @map("_id") @db.ObjectId
Post Post[]
}
"#,
);

@@unique([attribute], name: "composite_index")
}
assert!(res.is_ok());
}

type Attribute {
name String
value String
group String
}
#[test]
fn converting_composite_types_compound_unique() {
Copy link
Contributor

Choose a reason for hiding this comment

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

💯 to the exhaustive tests

let res = psl::parse_schema(
r#"
datasource db {
provider = "mongodb"
url = "mongodb://localhost:27017/hello"
}

model Post {
id String @id @default(auto()) @map("_id") @db.ObjectId
author User @relation(fields: [authorId], references: [id])
authorId String @db.ObjectId
attributes Attribute[]

@@unique([authorId, attributes])
// ^^^^^^^^^^^^^^^^^^^^^^
// Prisma does not currently support composite types in compound unique indices...
}

type Attribute {
name String
value String
group String
}

model User {
id String @id @default(auto()) @map("_id") @db.ObjectId
Post Post[]
}
"#,
);
assert!(res.unwrap_err().contains("Indexes can only contain scalar attributes. Please remove \"attribute\" from the argument list of the indexes."));

assert!(res
.unwrap_err()
.contains(r#"Prisma does not currently support composite types in compound unique indices, please remove "attributes" from the index. See https://pris.ly/d/mongodb-composite-compound-indices for more details"#));
}

#[test]
fn converting_composite_types_nested() {
let res = psl::parse_schema(
r#"
datasource db {
Druue marked this conversation as resolved.
Show resolved Hide resolved
provider = "mongodb"
url = "mongodb://localhost:27017/hello"
}

type TheatersLocation {
address TheatersLocationAddress
geo TheatersLocationGeo
}

type TheatersLocationAddress {
city String
state String
street1 String
street2 String?
zipcode String
}

type TheatersLocationGeo {
coordinates Float[]
type String
}

model theaters {
id String @id @default(auto()) @map("_id") @db.ObjectId
location TheatersLocation
theaterId Int

@@index([location.geo], map: "geo index")
}
"#,
);

assert!(res.is_ok());
}

#[test]
fn converting_composite_types_nested_scalar() {
let res = psl::parse_schema(
r#"
datasource db {
Druue marked this conversation as resolved.
Show resolved Hide resolved
provider = "mongodb"
url = "mongodb://localhost:27017/hello"
}

type TheatersLocation {
address TheatersLocationAddress
geo TheatersLocationGeo
}

type TheatersLocationAddress {
city String
state String
street1 String
street2 String?
zipcode String
}

type TheatersLocationGeo {
coordinates Float[]
type String
}

model theaters {
id String @id @default(auto()) @map("_id") @db.ObjectId
location TheatersLocation
theaterId Int

@@index([location.geo.type], map: "geo index")
}
"#,
);

assert!(res.is_ok());
}
// endregion

#[test]
fn models_with_only_scalar_fields() {
Expand Down
Loading