From 5dbef10bdbfb579e07d35cc85fb1518d357cb99e Mon Sep 17 00:00:00 2001 From: Alexey Orlenko Date: Thu, 28 Nov 2024 16:37:05 +0100 Subject: [PATCH] feat(schema-engine): use pk for implicit join tables on postgres (#5057) Co-authored-by: jacek-prisma --- .../datamodel_calculator/context.rs | 2 +- .../datamodel_calculator/context/flavour.rs | 4 + .../context/flavour/postgresql.rs | 4 + .../introspection/introspection_helpers.rs | 24 +- .../src/introspection/introspection_map.rs | 30 ++- .../introspection_map/relation_names.rs | 18 +- .../src/sql_schema_calculator.rs | 24 +- .../sql_schema_calculator_flavour.rs | 9 + .../sql_schema_calculator_flavour/postgres.rs | 10 +- .../src/sql_schema_differ.rs | 103 +++++++- .../sql_schema_differ_flavour/mysql.rs | 2 +- .../tests/relations/postgres.rs | 52 +++- .../tests/migrations/diff.rs | 227 ++++++++++++++++++ .../tests/migrations/indexes.rs | 46 ++++ .../tests/migrations/relations.rs | 19 +- ...join_table_is_in_first_model_schema.prisma | 7 +- 16 files changed, 536 insertions(+), 45 deletions(-) diff --git a/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context.rs b/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context.rs index 04dcfa7345de..ca97019df32b 100644 --- a/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context.rs +++ b/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context.rs @@ -108,7 +108,7 @@ impl<'a> DatamodelCalculatorContext<'a> { .table_walkers() .filter(|table| !is_old_migration_table(*table)) .filter(|table| !is_new_migration_table(*table)) - .filter(|table| !is_prisma_m_to_n_relation(*table)) + .filter(|table| !is_prisma_m_to_n_relation(*table, self.flavour.uses_pk_in_m2m_join_tables(self))) .filter(|table| !is_relay_table(*table)) .map(move |next| { let previous = self.existing_model(next.id); diff --git a/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour.rs b/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour.rs index cf18d313f375..6256d79aa8bf 100644 --- a/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour.rs +++ b/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour.rs @@ -61,4 +61,8 @@ pub(crate) trait IntrospectionFlavour { fn uses_exclude_constraint(&self, _ctx: &DatamodelCalculatorContext<'_>, _table: TableWalker<'_>) -> bool { false } + + fn uses_pk_in_m2m_join_tables(&self, _ctx: &DatamodelCalculatorContext<'_>) -> bool { + false + } } diff --git a/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour/postgresql.rs b/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour/postgresql.rs index 502dc985f4cb..d9a7a320eb7d 100644 --- a/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour/postgresql.rs +++ b/schema-engine/connectors/sql-schema-connector/src/introspection/datamodel_calculator/context/flavour/postgresql.rs @@ -96,4 +96,8 @@ impl super::IntrospectionFlavour for PostgresIntrospectionFlavour { let pg_ext: &PostgresSchemaExt = ctx.sql_schema.downcast_connector_data(); pg_ext.uses_exclude_constraint(table.id) } + + fn uses_pk_in_m2m_join_tables(&self, ctx: &DatamodelCalculatorContext<'_>) -> bool { + !ctx.is_cockroach() + } } diff --git a/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_helpers.rs b/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_helpers.rs index 5e7ada24ab36..9e68ca9ba7d1 100644 --- a/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_helpers.rs +++ b/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_helpers.rs @@ -1,7 +1,7 @@ //! Small utility functions. use sql::walkers::TableWalker; -use sql_schema_describer::{self as sql, IndexType}; +use sql_schema_describer::{self as sql, IndexColumnWalker, IndexType}; use std::cmp; /// This function implements the reverse behaviour of the `Ord` implementation for `Option`: it @@ -56,7 +56,7 @@ pub(crate) fn is_relay_table(table: TableWalker<'_>) -> bool { } /// If a relation defines a Prisma many to many relation. -pub(crate) fn is_prisma_m_to_n_relation(table: TableWalker<'_>) -> bool { +pub(crate) fn is_prisma_m_to_n_relation(table: TableWalker<'_>, pk_allowed: bool) -> bool { fn is_a(column: &str) -> bool { column.eq_ignore_ascii_case("a") } @@ -65,9 +65,18 @@ pub(crate) fn is_prisma_m_to_n_relation(table: TableWalker<'_>) -> bool { column.eq_ignore_ascii_case("b") } + fn index_columns_match<'a>(mut columns: impl ExactSizeIterator>) -> bool { + columns.len() == 2 + && match (columns.next(), columns.next()) { + (Some(a), Some(b)) => is_a(a.name()) && is_b(b.name()), + _ => false, + } + } + let mut fks = table.foreign_keys(); let first_fk = fks.next(); let second_fk = fks.next(); + let a_b_match = || { let first_fk = first_fk.unwrap(); let second_fk = second_fk.unwrap(); @@ -80,14 +89,13 @@ pub(crate) fn is_prisma_m_to_n_relation(table: TableWalker<'_>) -> bool { && is_b(first_fk_col) && is_a(second_fk_col)) }; + table.name().starts_with('_') - //UNIQUE INDEX [A,B] - && table.indexes().any(|i| { - i.columns().len() == 2 - && is_a(i.columns().next().unwrap().as_column().name()) - && is_b(i.columns().nth(1).unwrap().as_column().name()) + // UNIQUE INDEX (A, B) or PRIMARY KEY (A, B) + && (table.indexes().any(|i| { + index_columns_match(i.columns()) && i.is_unique() - }) + }) || pk_allowed && table.primary_key_columns().map(index_columns_match).unwrap_or(false)) //INDEX [B] && table .indexes() diff --git a/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map.rs b/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map.rs index 5fd5019213ac..a4351220bdaf 100644 --- a/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map.rs +++ b/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map.rs @@ -50,11 +50,11 @@ impl<'a> IntrospectionMap<'a> { match_enums(sql_schema, prisma_schema, &mut map); match_existing_scalar_fields(sql_schema, prisma_schema, &mut map); match_existing_inline_relations(sql_schema, prisma_schema, &mut map); - match_existing_m2m_relations(sql_schema, prisma_schema, &mut map); + match_existing_m2m_relations(sql_schema, prisma_schema, ctx, &mut map); relation_names::introspect(ctx, &mut map); - position_inline_relation_fields(sql_schema, &mut map); - position_m2m_relation_fields(sql_schema, &mut map); - populate_top_level_names(sql_schema, prisma_schema, &mut map); + position_inline_relation_fields(sql_schema, ctx, &mut map); + position_m2m_relation_fields(sql_schema, ctx, &mut map); + populate_top_level_names(sql_schema, prisma_schema, ctx, &mut map); map } @@ -63,11 +63,12 @@ impl<'a> IntrospectionMap<'a> { fn populate_top_level_names<'a>( sql_schema: &'a sql::SqlSchema, prisma_schema: &'a psl::ValidatedSchema, + ctx: &DatamodelCalculatorContext<'_>, map: &mut IntrospectionMap<'a>, ) { for table in sql_schema .table_walkers() - .filter(|t| !helpers::is_prisma_m_to_n_relation(*t)) + .filter(|t| !helpers::is_prisma_m_to_n_relation(*t, ctx.flavour.uses_pk_in_m2m_join_tables(ctx))) { let name = map .existing_models @@ -115,10 +116,14 @@ fn populate_top_level_names<'a>( /// Inlined relation fields (foreign key is defined in a model) are /// sorted in a specific way. We handle the sorting here. -fn position_inline_relation_fields(sql_schema: &sql::SqlSchema, map: &mut IntrospectionMap<'_>) { +fn position_inline_relation_fields( + sql_schema: &sql::SqlSchema, + ctx: &DatamodelCalculatorContext<'_>, + map: &mut IntrospectionMap<'_>, +) { for table in sql_schema .table_walkers() - .filter(|t| !helpers::is_prisma_m_to_n_relation(*t)) + .filter(|t| !helpers::is_prisma_m_to_n_relation(*t, ctx.flavour.uses_pk_in_m2m_join_tables(ctx))) { for fk in table.foreign_keys() { map.inline_relation_positions @@ -133,10 +138,14 @@ fn position_inline_relation_fields(sql_schema: &sql::SqlSchema, map: &mut Intros /// Many to many relation fields (foreign keys are defined in a hidden /// join table) are sorted in a specific way. We handle the sorting /// here. -fn position_m2m_relation_fields(sql_schema: &sql::SqlSchema, map: &mut IntrospectionMap<'_>) { +fn position_m2m_relation_fields( + sql_schema: &sql::SqlSchema, + ctx: &DatamodelCalculatorContext<'_>, + map: &mut IntrospectionMap<'_>, +) { for table in sql_schema .table_walkers() - .filter(|t| helpers::is_prisma_m_to_n_relation(*t)) + .filter(|t| helpers::is_prisma_m_to_n_relation(*t, ctx.flavour.uses_pk_in_m2m_join_tables(ctx))) { let mut fks = table.foreign_keys(); @@ -313,11 +322,12 @@ fn match_existing_inline_relations<'a>( fn match_existing_m2m_relations( sql_schema: &sql::SqlSchema, prisma_schema: &psl::ValidatedSchema, + ctx: &DatamodelCalculatorContext<'_>, map: &mut IntrospectionMap<'_>, ) { map.existing_m2m_relations = sql_schema .table_walkers() - .filter(|t| helpers::is_prisma_m_to_n_relation(*t)) + .filter(|t| helpers::is_prisma_m_to_n_relation(*t, ctx.flavour.uses_pk_in_m2m_join_tables(ctx))) .filter_map(|table| { prisma_schema .db diff --git a/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map/relation_names.rs b/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map/relation_names.rs index 75ee5f9a8099..70ed3ae51f40 100644 --- a/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map/relation_names.rs +++ b/schema-engine/connectors/sql-schema-connector/src/introspection/introspection_map/relation_names.rs @@ -74,7 +74,7 @@ pub(super) fn introspect<'a>(ctx: &DatamodelCalculatorContext<'a>, map: &mut sup let ambiguous_relations = find_ambiguous_relations(ctx); for table in ctx.sql_schema.table_walkers() { - if is_prisma_m_to_n_relation(table) { + if is_prisma_m_to_n_relation(table, ctx.flavour.uses_pk_in_m2m_join_tables(ctx)) { let name = prisma_m2m_relation_name(table, &ambiguous_relations, ctx); names.m2m_relation_names.insert(table.id, name); } else { @@ -175,8 +175,8 @@ fn find_ambiguous_relations(ctx: &DatamodelCalculatorContext<'_>) -> HashSet<[sq let mut ambiguous_relations = HashSet::new(); for table in ctx.sql_schema.table_walkers() { - if is_prisma_m_to_n_relation(table) { - m2m_relation_ambiguousness(table, &mut ambiguous_relations) + if is_prisma_m_to_n_relation(table, ctx.flavour.uses_pk_in_m2m_join_tables(ctx)) { + m2m_relation_ambiguousness(table, ctx, &mut ambiguous_relations) } else { for fk in table.foreign_keys() { inline_relation_ambiguousness(fk, &mut ambiguous_relations, ctx) @@ -187,7 +187,11 @@ fn find_ambiguous_relations(ctx: &DatamodelCalculatorContext<'_>) -> HashSet<[sq ambiguous_relations } -fn m2m_relation_ambiguousness(table: sql::TableWalker<'_>, ambiguous_relations: &mut HashSet<[sql::TableId; 2]>) { +fn m2m_relation_ambiguousness( + table: sql::TableWalker<'_>, + ctx: &DatamodelCalculatorContext<'_>, + ambiguous_relations: &mut HashSet<[sql::TableId; 2]>, +) { let tables = table_ids_for_m2m_relation_table(table); if ambiguous_relations.contains(&tables) { @@ -205,7 +209,11 @@ fn m2m_relation_ambiguousness(table: sql::TableWalker<'_>, ambiguous_relations: } // Check for conflicts with another m2m relation. - for other_m2m in table.schema.table_walkers().filter(|t| is_prisma_m_to_n_relation(*t)) { + for other_m2m in table + .schema + .table_walkers() + .filter(|t| is_prisma_m_to_n_relation(*t, ctx.flavour.uses_pk_in_m2m_join_tables(ctx))) + { if other_m2m.id != table.id && table_ids_for_m2m_relation_table(other_m2m) == tables { ambiguous_relations.insert(tables); } diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator.rs b/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator.rs index fcbe39bccf02..eb66f4ce5b78 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator.rs @@ -1,5 +1,6 @@ mod sql_schema_calculator_flavour; +use sql_schema_calculator_flavour::JoinTableUniquenessConstraint; pub(super) use sql_schema_calculator_flavour::SqlSchemaCalculatorFlavour; use crate::{flavour::SqlFlavour, SqlDatabaseSchema}; @@ -12,7 +13,7 @@ use psl::{ }, ValidatedSchema, }; -use sql_schema_describer::{self as sql, PrismaValue}; +use sql_schema_describer::{self as sql, PrismaValue, SqlSchema}; use std::collections::HashMap; pub(crate) fn calculate_sql_schema(datamodel: &ValidatedSchema, flavour: &dyn SqlFlavour) -> SqlDatabaseSchema { @@ -261,13 +262,24 @@ fn push_relation_tables(ctx: &mut Context<'_>) { }, ); - // Unique index on AB + // Unique index or PK on AB { - let index_name = format!( - "{}_AB_unique", - table_name.chars().take(max_identifier_length - 10).collect::() + let (constraint_suffix, push_constraint): (_, fn(_, _, _) -> _) = + match ctx.flavour.m2m_join_table_constraint() { + JoinTableUniquenessConstraint::PrimaryKey => ("_AB_pkey", SqlSchema::push_primary_key), + JoinTableUniquenessConstraint::UniqueIndex => ("_AB_unique", SqlSchema::push_unique_constraint), + }; + + let constraint_name = format!( + "{}{constraint_suffix}", + table_name + .chars() + .take(max_identifier_length - constraint_suffix.len()) + .collect::() ); - let index_id = ctx.schema.describer_schema.push_unique_constraint(table_id, index_name); + + let index_id = push_constraint(&mut ctx.schema.describer_schema, table_id, constraint_name); + ctx.schema.describer_schema.push_index_column(sql::IndexColumn { index_id, column_id: column_a_id, diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour.rs b/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour.rs index 6af43769ed74..2112e44fceed 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour.rs @@ -39,4 +39,13 @@ pub(crate) trait SqlSchemaCalculatorFlavour { } fn push_connector_data(&self, _context: &mut super::Context<'_>) {} + + fn m2m_join_table_constraint(&self) -> JoinTableUniquenessConstraint { + JoinTableUniquenessConstraint::UniqueIndex + } +} + +pub(crate) enum JoinTableUniquenessConstraint { + PrimaryKey, + UniqueIndex, } diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour/postgres.rs b/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour/postgres.rs index c2193252be99..017f018733dc 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour/postgres.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_schema_calculator/sql_schema_calculator_flavour/postgres.rs @@ -1,4 +1,4 @@ -use super::{super::Context, SqlSchemaCalculatorFlavour}; +use super::{super::Context, JoinTableUniquenessConstraint, SqlSchemaCalculatorFlavour}; use crate::flavour::{PostgresFlavour, SqlFlavour}; use either::Either; use psl::{ @@ -162,6 +162,14 @@ impl SqlSchemaCalculatorFlavour for PostgresFlavour { .describer_schema .set_connector_data(Box::new(postgres_ext)); } + + fn m2m_join_table_constraint(&self) -> JoinTableUniquenessConstraint { + if self.is_cockroachdb() { + JoinTableUniquenessConstraint::UniqueIndex + } else { + JoinTableUniquenessConstraint::PrimaryKey + } + } } fn convert_opclass(opclass: OperatorClass, algo: Option) -> sql::postgres::SQLOperatorClass { diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs b/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs index 31d237fd99be..feccbb0f87c2 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ.rs @@ -16,7 +16,7 @@ use crate::{ SqlFlavour, }; use column::ColumnTypeChange; -use sql_schema_describer::{walkers::ForeignKeyWalker, IndexId, TableColumnId}; +use sql_schema_describer::{walkers::ForeignKeyWalker, IndexId, TableColumnId, Walker}; use std::{borrow::Cow, collections::HashSet}; use table::TableDiffer; @@ -40,7 +40,7 @@ pub(crate) fn calculate_steps( flavour.push_enum_steps(&mut steps, &db); flavour.push_alter_sequence_steps(&mut steps, &db); - steps.sort(); + sort_migration_steps(&mut steps, &db); steps } @@ -315,7 +315,7 @@ fn push_alter_primary_key(differ: &TableDiffer<'_, '_>, steps: &mut Vec return, }; - if all_match(&mut previous.column_names(), &mut next.column_names()) { + if all_match(previous.column_names(), next.column_names()) { return; } @@ -538,6 +538,101 @@ fn is_prisma_implicit_m2m_fk(fk: ForeignKeyWalker<'_>) -> bool { table.column("A").is_some() && table.column("B").is_some() } -fn all_match(a: &mut dyn ExactSizeIterator, b: &mut dyn ExactSizeIterator) -> bool { +fn all_match(a: impl ExactSizeIterator, b: impl ExactSizeIterator) -> bool { a.len() == b.len() && a.zip(b).all(|(a, b)| a == b) } + +fn sort_migration_steps(steps: &mut Vec, db: &DifferDatabase<'_>) { + // We can't merge these two steps into `sort_by` because sorting algorithms require total order + // relation, but the dependency between dropped unique index and creating a corresponding + // primary key is a preorder. Moreover, the binary relation defined as the composition of + // custom logic for `(SqlMigrationStep::DropIndex, SqlMigrationStep::AlterTable)` pairs and + // `a.cmp(b)` for everything else doesn't appear to be even transitive (due to the existing + // automatically derived total order relation between `SqlMigrationStep`s). If we need more + // complex ordering logic in the future, we should consider defining a partial order on + // `SqlMigrationStep` where only the pairs for which order actually matters are ordered, + // building a graph from the steps and topologically sorting it. + steps.sort(); + apply_partial_order_permutations(steps, db); +} + +fn apply_partial_order_permutations(steps: &mut Vec, db: &DifferDatabase<'_>) { + fn find_dropped_unique_index<'a>( + steps: &[SqlMigrationStep], + seen_elements: &mut usize, + db: &DifferDatabase<'a>, + ) -> Option> { + for step in steps { + *seen_elements += 1; + + if let SqlMigrationStep::DropIndex { index_id } = step { + let index = db.schemas.previous.describer_schema.walk(*index_id); + + // We're interested in dropped unique indexes in tables that didn't have a primary + // key defined. + if index.is_unique() && index.table().primary_key().is_none() { + return Some(index); + } + } + } + + None + } + + fn find_matching_created_pk_step<'a>( + steps: &[SqlMigrationStep], + index: Walker<'a, IndexId>, + db: &DifferDatabase<'a>, + ) -> Option { + steps + .iter() + .enumerate() + .filter_map(|(i, step)| match step { + SqlMigrationStep::AlterTable(alter_table) => Some((i, alter_table)), + _ => None, + }) + .filter(|(_, alter_table)| alter_table.table_ids.previous == index.table().id) + // We're only interested in `AlterTable` steps that create a primary key. + .filter(|(_, alter_table)| { + alter_table + .changes + .iter() + .any(|change| matches!(change, TableChange::AddPrimaryKey)) + }) + // This `AlterTable` step must not have dropped or recreated any columns from the + // unique index we were looking at. + .filter(|(_, alter_table)| { + alter_table.changes.iter().all(|change| match change { + TableChange::DropColumn { column_id } => !index.contains_column(*column_id), + TableChange::DropAndRecreateColumn { column_id, .. } => !index.contains_column(column_id.previous), + _ => true, + }) + }) + // The primary key must be created on the same columns as the unique index. + .find(|(_, alter_table)| { + let table = db.schemas.next.describer_schema.walk(alter_table.table_ids.next); + table.primary_key().is_some() + && all_match( + index.column_names(), + table.primary_key_columns().unwrap().map(|col| col.name()), + ) + }) + .map(|(i, _)| i) + } + + let mut i = 0; + + while let Some(index) = find_dropped_unique_index(&steps[i..], &mut i, db) { + let index_pos = i - 1; + + if let Some(alter_table_offset) = find_matching_created_pk_step(&steps[i..], index, db) { + let alter_table_pos = i + alter_table_offset; + let drop_index_step = steps.remove(index_pos); + steps.insert(alter_table_pos, drop_index_step); + + // We need to adjust the index so we don't skip the element following the `DropIndex` + // step we just moved, as the following elements were shifted left by one. + i -= 1; + } + } +} diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ/sql_schema_differ_flavour/mysql.rs b/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ/sql_schema_differ_flavour/mysql.rs index 4f4dea5c7a45..5e8ad42e10ad 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ/sql_schema_differ_flavour/mysql.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_schema_differ/sql_schema_differ_flavour/mysql.rs @@ -44,7 +44,7 @@ impl SqlSchemaDifferFlavour for MysqlFlavour { differ.next.column_type_family_as_enum(), ) { (Some(previous_enum), Some(next_enum)) => { - if all_match(&mut previous_enum.values(), &mut next_enum.values()) { + if all_match(previous_enum.values(), next_enum.values()) { return None; } diff --git a/schema-engine/sql-introspection-tests/tests/relations/postgres.rs b/schema-engine/sql-introspection-tests/tests/relations/postgres.rs index 56fb1d497c1c..d3ee2cd3fafb 100644 --- a/schema-engine/sql-introspection-tests/tests/relations/postgres.rs +++ b/schema-engine/sql-introspection-tests/tests/relations/postgres.rs @@ -218,7 +218,7 @@ async fn name_ambiguity_with_a_scalar_field(api: &mut TestApi) -> TestResult { } #[test_connector(tags(Postgres), exclude(CockroachDb))] -async fn a_prisma_many_to_many_relation(api: &mut TestApi) -> TestResult { +async fn legacy_prisma_many_to_many_relation(api: &mut TestApi) -> TestResult { let setup = indoc! {r#" CREATE TABLE "User" ( id SERIAL PRIMARY KEY @@ -266,3 +266,53 @@ async fn a_prisma_many_to_many_relation(api: &mut TestApi) -> TestResult { Ok(()) } + +#[test_connector(tags(Postgres), exclude(CockroachDb))] +async fn new_prisma_many_to_many_relation(api: &mut TestApi) -> TestResult { + let setup = indoc! {r#" + CREATE TABLE "User" ( + id SERIAL PRIMARY KEY + ); + + CREATE TABLE "Post" ( + id SERIAL PRIMARY KEY + ); + + CREATE TABLE "_PostToUser" ( + "A" INT NOT NULL, + "B" INT NOT NULL, + CONSTRAINT "_PostToUser_A_fkey" FOREIGN KEY ("A") REFERENCES "Post"(id), + CONSTRAINT "_PostToUser_B_fkey" FOREIGN KEY ("B") REFERENCES "User"(id), + CONSTRAINT "_PostToUser_AB_pkey" PRIMARY KEY ("A", "B") + ); + + CREATE INDEX test ON "_PostToUser" ("B"); + "#}; + + api.raw_cmd(setup).await; + + let expected = expect![[r#" + generator client { + provider = "prisma-client-js" + } + + datasource db { + provider = "postgresql" + url = "env(TEST_DATABASE_URL)" + } + + model Post { + id Int @id @default(autoincrement()) + User User[] + } + + model User { + id Int @id @default(autoincrement()) + Post Post[] + } + "#]]; + + api.expect_datamodel(&expected).await; + + Ok(()) +} diff --git a/schema-engine/sql-migration-tests/tests/migrations/diff.rs b/schema-engine/sql-migration-tests/tests/migrations/diff.rs index b9225bd7a22d..d9fab5e0470e 100644 --- a/schema-engine/sql-migration-tests/tests/migrations/diff.rs +++ b/schema-engine/sql-migration-tests/tests/migrations/diff.rs @@ -96,6 +96,233 @@ fn from_unique_index_to_without(mut api: TestApi) { expected_printed_messages.assert_debug_eq(&host.printed_messages.lock().unwrap()); } +#[test_connector] +fn from_unique_index_to_pk(mut api: TestApi) { + let tempdir = tempfile::tempdir().unwrap(); + let host = Arc::new(TestConnectorHost::default()); + + api.connector.set_host(host.clone()); + + let from_schema = api.datamodel_with_provider( + r#" + model A { + id Int @unique + name String? + } + + model B { + x Int + y Int + + @@unique([x, y]) + } + + model C { + id Int @id + secondary Int @unique + } + "#, + ); + + let to_schema = api.datamodel_with_provider( + r#" + model A { + id Int @id + } + + model B { + x Int + y Int + + @@id([x, y]) + } + + // Dropping the unique index on `secondary` will not be reordered + // so it will appear before the modifications on A and B in the migration. + model C { + id Int @id + secondary Int + } + "#, + ); + + let from_file = write_file_to_tmp(&from_schema, &tempdir, "from"); + let to_file = write_file_to_tmp(&to_schema, &tempdir, "to"); + + api.diff(DiffParams { + exit_code: None, + from: DiffTarget::SchemaDatamodel(SchemasContainer { + files: vec![SchemaContainer { + path: from_file.to_string_lossy().into_owned(), + content: from_schema.to_string(), + }], + }), + shadow_database_url: None, + to: DiffTarget::SchemaDatamodel(SchemasContainer { + files: vec![SchemaContainer { + path: to_file.to_string_lossy().into_owned(), + content: to_schema.to_string(), + }], + }), + script: true, + }) + .unwrap(); + + let expected_printed_messages = if api.is_mysql() { + expect![[r#" + [ + [ + "-- DropIndex", + "DROP INDEX `C_secondary_key` ON `C`;", + "", + "-- AlterTable", + "ALTER TABLE `A` DROP COLUMN `name`,", + " ADD PRIMARY KEY (`id`);", + "", + "-- DropIndex", + "DROP INDEX `A_id_key` ON `A`;", + "", + "-- AlterTable", + "ALTER TABLE `B` ADD PRIMARY KEY (`x`, `y`);", + "", + "-- DropIndex", + "DROP INDEX `B_x_y_key` ON `B`;", + "", + ], + ] + "#]] + } else if api.is_postgres() && !api.is_cockroach() { + expect![[r#" + [ + [ + "-- DropIndex", + "DROP INDEX \"C_secondary_key\";", + "", + "-- AlterTable", + "ALTER TABLE \"A\" DROP COLUMN \"name\",", + "ADD CONSTRAINT \"A_pkey\" PRIMARY KEY (\"id\");", + "", + "-- DropIndex", + "DROP INDEX \"A_id_key\";", + "", + "-- AlterTable", + "ALTER TABLE \"B\" ADD CONSTRAINT \"B_pkey\" PRIMARY KEY (\"x\", \"y\");", + "", + "-- DropIndex", + "DROP INDEX \"B_x_y_key\";", + "", + ], + ] + "#]] + } else if api.is_cockroach() { + expect![[r#" + [ + [ + "-- DropIndex", + "DROP INDEX \"C_secondary_key\";", + "", + "-- AlterTable", + "ALTER TABLE \"A\" DROP COLUMN \"name\";", + "ALTER TABLE \"A\" ADD CONSTRAINT \"A_pkey\" PRIMARY KEY (\"id\");", + "", + "-- DropIndex", + "DROP INDEX \"A_id_key\";", + "", + "-- AlterTable", + "ALTER TABLE \"B\" ADD CONSTRAINT \"B_pkey\" PRIMARY KEY (\"x\", \"y\");", + "", + "-- DropIndex", + "DROP INDEX \"B_x_y_key\";", + "", + ], + ] + "#]] + } else if api.is_mssql() { + expect![[r#" + [ + [ + "BEGIN TRY", + "", + "BEGIN TRAN;", + "", + "-- DropIndex", + "DROP INDEX [C_secondary_key] ON [dbo].[C];", + "", + "-- AlterTable", + "ALTER TABLE [dbo].[A] DROP COLUMN [name];", + "ALTER TABLE [dbo].[A] ADD CONSTRAINT A_pkey PRIMARY KEY CLUSTERED ([id]);", + "", + "-- DropIndex", + "DROP INDEX [A_id_key] ON [dbo].[A];", + "", + "-- AlterTable", + "ALTER TABLE [dbo].[B] ADD CONSTRAINT B_pkey PRIMARY KEY CLUSTERED ([x],[y]);", + "", + "-- DropIndex", + "DROP INDEX [B_x_y_key] ON [dbo].[B];", + "", + "COMMIT TRAN;", + "", + "END TRY", + "BEGIN CATCH", + "", + "IF @@TRANCOUNT > 0", + "BEGIN", + " ROLLBACK TRAN;", + "END;", + "THROW", + "", + "END CATCH", + "", + ], + ] + "#]] + } else if api.is_sqlite() { + expect![[r#" + [ + [ + "-- DropIndex", + "DROP INDEX \"C_secondary_key\";", + "", + "-- RedefineTables", + "PRAGMA defer_foreign_keys=ON;", + "PRAGMA foreign_keys=OFF;", + "CREATE TABLE \"new_A\" (", + " \"id\" INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT", + ");", + "INSERT INTO \"new_A\" (\"id\") SELECT \"id\" FROM \"A\";", + "DROP TABLE \"A\";", + "ALTER TABLE \"new_A\" RENAME TO \"A\";", + "CREATE TABLE \"new_B\" (", + " \"x\" INTEGER NOT NULL,", + " \"y\" INTEGER NOT NULL,", + "", + " PRIMARY KEY (\"x\", \"y\")", + ");", + "INSERT INTO \"new_B\" (\"x\", \"y\") SELECT \"x\", \"y\" FROM \"B\";", + "DROP TABLE \"B\";", + "ALTER TABLE \"new_B\" RENAME TO \"B\";", + "PRAGMA foreign_keys=ON;", + "PRAGMA defer_foreign_keys=OFF;", + "", + ], + ] + "#]] + } else { + unreachable!() + }; + + expected_printed_messages.assert_debug_eq( + &host + .printed_messages + .lock() + .unwrap() + .iter() + .map(|msg| msg.split('\n').map(<_>::to_owned).collect()) + .collect::>>(), + ); +} + #[test_connector(tags(Sqlite))] fn diffing_postgres_schemas_when_initialized_on_sqlite(mut api: TestApi) { // We should get a postgres diff. diff --git a/schema-engine/sql-migration-tests/tests/migrations/indexes.rs b/schema-engine/sql-migration-tests/tests/migrations/indexes.rs index 9b534349c376..a8c140f3f3e1 100644 --- a/schema-engine/sql-migration-tests/tests/migrations/indexes.rs +++ b/schema-engine/sql-migration-tests/tests/migrations/indexes.rs @@ -975,3 +975,49 @@ fn changing_normal_index_to_a_fulltext_index(api: TestApi) { table.assert_index_on_columns(&["a", "b"], |index| index.assert_is_fulltext()) }); } + +#[test_connector] +fn changing_unique_to_pk_works(api: TestApi) { + let dm1 = indoc! {r#" + model A { + id Int @unique + name String? + } + + model B { + x Int + y Int + + @@unique([x, y]) + } + "#}; + + api.schema_push_w_datasource(dm1).send().assert_green(); + + api.assert_schema() + .assert_table("A", |table| { + table.assert_index_on_columns(&["id"], |index| index.assert_is_unique()) + }) + .assert_table("B", |table| { + table.assert_index_on_columns(&["x", "y"], |index| index.assert_is_unique()) + }); + + let dm2 = indoc! {r#" + model A { + id Int @id + } + + model B { + x Int + y Int + + @@id([x, y]) + } + "#}; + + api.schema_push_w_datasource(dm2).send().assert_green(); + + api.assert_schema() + .assert_table("A", |table| table.assert_pk(|pk| pk.assert_columns(&["id"]))) + .assert_table("B", |table| table.assert_pk(|pk| pk.assert_columns(&["x", "y"]))); +} diff --git a/schema-engine/sql-migration-tests/tests/migrations/relations.rs b/schema-engine/sql-migration-tests/tests/migrations/relations.rs index d58e9efd8590..4702bf11d8c8 100644 --- a/schema-engine/sql-migration-tests/tests/migrations/relations.rs +++ b/schema-engine/sql-migration-tests/tests/migrations/relations.rs @@ -992,7 +992,11 @@ fn migrations_with_many_to_many_related_models_must_not_recreate_indexes(api: Te api.schema_push_w_datasource(dm_1).send().assert_green(); api.assert_schema().assert_table("_ProfileToSkill", |t| { - t.assert_index_on_columns(&["A", "B"], |idx| idx.assert_is_unique()) + if api.is_postgres() && !api.is_cockroach() { + t.assert_pk(|pk| pk.assert_columns(&["A", "B"])) + } else { + t.assert_index_on_columns(&["A", "B"], |idx| idx.assert_is_unique()) + } }); let dm_2 = r#" @@ -1017,9 +1021,16 @@ fn migrations_with_many_to_many_related_models_must_not_recreate_indexes(api: Te api.schema_push_w_datasource(dm_2).send().assert_green(); api.assert_schema().assert_table("_ProfileToSkill", |table| { - table.assert_index_on_columns(&["A", "B"], |idx| { - idx.assert_is_unique().assert_name("_ProfileToSkill_AB_unique") - }) + if api.is_postgres() && !api.is_cockroach() { + table.assert_pk(|pk| { + pk.assert_columns(&["A", "B"]) + .assert_constraint_name("_ProfileToSkill_AB_pkey") + }) + } else { + table.assert_index_on_columns(&["A", "B"], |idx| { + idx.assert_is_unique().assert_name("_ProfileToSkill_AB_unique") + }) + } }); // Check that the migration is idempotent diff --git a/schema-engine/sql-migration-tests/tests/single_migration_tests/postgres/multi_schema_implicit_many_to_many_join_table_is_in_first_model_schema.prisma b/schema-engine/sql-migration-tests/tests/single_migration_tests/postgres/multi_schema_implicit_many_to_many_join_table_is_in_first_model_schema.prisma index ec691474cc80..51db3cec5fa1 100644 --- a/schema-engine/sql-migration-tests/tests/single_migration_tests/postgres/multi_schema_implicit_many_to_many_join_table_is_in_first_model_schema.prisma +++ b/schema-engine/sql-migration-tests/tests/single_migration_tests/postgres/multi_schema_implicit_many_to_many_join_table_is_in_first_model_schema.prisma @@ -48,11 +48,10 @@ model shirataki { // -- CreateTable // CREATE TABLE "roots"."_shiratakiTozoodles" ( // "A" INTEGER NOT NULL, -// "B" INTEGER NOT NULL -// ); +// "B" INTEGER NOT NULL, // -// -- CreateIndex -// CREATE UNIQUE INDEX "_shiratakiTozoodles_AB_unique" ON "roots"."_shiratakiTozoodles"("A", "B"); +// CONSTRAINT "_shiratakiTozoodles_AB_pkey" PRIMARY KEY ("A","B") +// ); // // -- CreateIndex // CREATE INDEX "_shiratakiTozoodles_B_index" ON "roots"."_shiratakiTozoodles"("B");