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

psl: fix database name clash validation with multiSchema #3255

Merged
merged 1 commit into from
Sep 30, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion psl/parser-database/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ fn resolve_model_attributes<'db>(model_id: ast::ModelId, ast_model: &'db ast::Mo

// @@map
if ctx.visit_optional_single_attr("map") {
map::model(&mut model_attributes, model_id, ctx);
map::model(&mut model_attributes, ctx);
ctx.validate_visited_arguments();
}

Expand Down
20 changes: 1 addition & 19 deletions psl/parser-database/src/attributes/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,13 @@ use crate::{
DatamodelError, StringId,
};

pub(super) fn model(model_attributes: &mut ModelAttributes, model_id: ast::ModelId, ctx: &mut Context<'_>) {
pub(super) fn model(model_attributes: &mut ModelAttributes, ctx: &mut Context<'_>) {
let mapped_name = match visit_map_attribute(ctx) {
Some(name) => name,
None => return,
};

model_attributes.mapped_name = Some(mapped_name);

if let Some(existing_model_id) = ctx.mapped_model_names.insert(mapped_name, model_id) {
let existing_model_name = ctx.ast[existing_model_id].name();
ctx.push_error(DatamodelError::new_duplicate_model_database_name_error(
&ctx[mapped_name],
existing_model_name,
ctx.ast[model_id].span(),
));
}

if let Some(existing_model_id) = ctx.names.tops.get(&mapped_name).and_then(|id| id.as_model_id()) {
let existing_model_name = ctx.ast[existing_model_id].name();
ctx.push_error(DatamodelError::new_duplicate_model_database_name_error(
&ctx[mapped_name],
existing_model_name,
ctx.current_attribute().span,
));
}
}

pub(super) fn scalar_field(
Expand Down
2 changes: 0 additions & 2 deletions psl/parser-database/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ pub(crate) struct Context<'db> {
attributes: AttributesValidationState, // state machine for attribute validation

// @map'ed names indexes. These are not in the db because they are only used for validation.
pub(super) mapped_model_names: HashMap<StringId, ast::ModelId>,
pub(super) mapped_model_scalar_field_names: HashMap<(ast::ModelId, StringId), ast::FieldId>,
pub(super) mapped_composite_type_names: HashMap<(ast::CompositeTypeId, StringId), ast::FieldId>,
pub(super) mapped_enum_names: HashMap<StringId, ast::EnumId>,
Expand All @@ -55,7 +54,6 @@ impl<'db> Context<'db> {
diagnostics,
attributes: AttributesValidationState::default(),

mapped_model_names: Default::default(),
mapped_model_scalar_field_names: Default::default(),
mapped_enum_names: Default::default(),
mapped_enum_value_names: Default::default(),
Expand Down
2 changes: 1 addition & 1 deletion psl/parser-database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl ParserDatabase {
&self.ast
}

/// The total number of models.
/// The total number of models. This is O(1).
pub fn models_count(&self) -> usize {
self.types.model_attributes.len()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ pub(super) fn validate(ctx: &mut Context<'_>) {
}
}

// Model validations
ctx.connector
.validate_scalar_field_unknown_default_functions(ctx.db, ctx.diagnostics);

// Model validations
models::database_name_clashes(ctx);

for model in db.walk_models() {
models::has_a_strict_unique_criteria(model, ctx);
models::has_a_unique_primary_key_name(model, &names, ctx);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use super::database_name::validate_db_name;
use crate::{
ast,
common::preview_features::PreviewFeature,
datamodel_connector::{walker_ext_traits::*, ConnectorCapability},
diagnostics::DatamodelError,
parser_database::ast::WithSpan,
parser_database::ast::{WithName, WithSpan},
validate::validation_pipeline::context::Context,
};
use parser_database::walkers::{ModelWalker, PrimaryKeyWalker};
use std::borrow::Cow;
use std::{borrow::Cow, collections::HashMap};

/// A model must have either a primary key, or a unique criteria
/// with no optional, commented-out or unsupported fields.
Expand Down Expand Up @@ -298,3 +299,46 @@ pub(super) fn schema_attribute(model: ModelWalker<'_>, ctx: &mut Context<'_>) {
_ => (),
}
}

pub(super) fn database_name_clashes(ctx: &mut Context<'_>) {
// (schema_name, model_database_name) -> ModelId
let mut database_names: HashMap<(Option<&str>, &str), ast::ModelId> = HashMap::with_capacity(ctx.db.models_count());

for model in ctx.db.walk_models() {
let key = (model.schema().map(|(name, _)| name), model.database_name());
match database_names.insert(key, model.model_id()) {
// Two branches because we want to put the error on the @@map attribute, and it can be
// on either model.
Some(existing) if model.mapped_name().is_some() => {
let existing_model_name = &ctx.db.ast()[existing].name();
let attribute = model
.ast_model()
.attributes
.iter()
.find(|attr| attr.name() == "map")
.unwrap();

ctx.push_error(DatamodelError::new_duplicate_model_database_name_error(
model.database_name(),
existing_model_name,
attribute.span(),
));
}
Some(existing) => {
let existing_model = &ctx.db.ast()[existing];
let attribute = existing_model
.attributes
.iter()
.find(|attr| attr.name() == "map")
.unwrap();

ctx.push_error(DatamodelError::new_duplicate_model_database_name_error(
model.database_name(),
model.name(),
attribute.span(),
));
}
None => (),
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
datasource mydb {
provider = "sqlite"
url = env("TEST_DB_URL")
}

model Dog {
id Int @id

@@map("pets")
}

model Cat {
id Int @id

@@map("pets")
}

// error: The model with database name "pets" could not be defined because another model with this name exists: "Dog"
// --> schema.prisma:15
//  | 
// 14 | 
// 15 |  @@map("pets")
//  | 
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// issue: https://github.com/prisma/prisma/issues/15009

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

datasource db {
provider = "postgresql"
url = env("TEST_DATABASE_URL")
schemas = ["base", "transactional"]
}

model User {
id String @id
email String
posts Post[]

@@map("some_table")
@@schema("base")
}

model Post {
title String
authorId String @unique
author User? @relation(fields: [authorId], references: [id])

@@map("some_table")
@@schema("transactional")
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
datasource mydb {
provider = "sqlserver"
url = env("TEST_DB_URL")
schemas = ["base", "transactional"]
}

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


model Dog {
id Int @id

@@map("pets")
@@schema("base")
}

model Cat {
id Int @id(map: "cat_pets_pkey")

@@map("pets")
@@schema("base")
}

// error: The model with database name "pets" could not be defined because another model with this name exists: "Dog"
// --> schema.prisma:23
//  | 
// 22 | 
// 23 |  @@map("pets")
//  |