From 1bd04d3c4463b74dc9f945abddf6eac276b660dd Mon Sep 17 00:00:00 2001 From: Juan Aguilar Santillana Date: Fri, 6 Sep 2024 00:45:42 +0200 Subject: [PATCH] fix: alter table postgres miss render non-action --- .../src/sql_renderer/postgres_renderer.rs | 46 ++++++++++------- .../tests/migrations/postgres.rs | 50 +++++++++++++++++++ 2 files changed, 77 insertions(+), 19 deletions(-) diff --git a/schema-engine/connectors/sql-schema-connector/src/sql_renderer/postgres_renderer.rs b/schema-engine/connectors/sql-schema-connector/src/sql_renderer/postgres_renderer.rs index fdebc14f89b2..5b883ca7c1c6 100644 --- a/schema-engine/connectors/sql-schema-connector/src/sql_renderer/postgres_renderer.rs +++ b/schema-engine/connectors/sql-schema-connector/src/sql_renderer/postgres_renderer.rs @@ -230,23 +230,24 @@ impl SqlRenderer for PostgresFlavour { fn render_alter_table(&self, alter_table: &AlterTable, schemas: MigrationPair<&SqlSchema>) -> Vec { let AlterTable { changes, table_ids } = alter_table; - let mut lines = Vec::new(); + let mut actions = Vec::new(); + let mut stats = Vec::new(); let mut before_statements = Vec::new(); let mut after_statements = Vec::new(); let tables = schemas.walk(*table_ids); for change in changes { match change { - TableChange::DropPrimaryKey => lines.push(format!( + TableChange::DropPrimaryKey => actions.push(format!( "DROP CONSTRAINT {}", Quoted::postgres_ident(tables.previous.primary_key().unwrap().name()) )), - TableChange::RenamePrimaryKey => lines.push(format!( + TableChange::RenamePrimaryKey => stats.push(format!( "RENAME CONSTRAINT {} TO {}", Quoted::postgres_ident(tables.previous.primary_key().unwrap().name()), Quoted::postgres_ident(tables.next.primary_key().unwrap().name()) )), - TableChange::AddPrimaryKey => lines.push({ + TableChange::AddPrimaryKey => actions.push({ let named = match tables.next.primary_key().map(|pk| pk.name()) { Some(name) => format!("CONSTRAINT {} ", self.quote(name)), None => "".into(), @@ -270,11 +271,11 @@ impl SqlRenderer for PostgresFlavour { let column = schemas.next.walk(*column_id); let col_sql = self.render_column(column); - lines.push(format!("ADD COLUMN {col_sql}")); + actions.push(format!("ADD COLUMN {col_sql}")); } TableChange::DropColumn { column_id } => { let name = self.quote(schemas.previous.walk(*column_id).name()); - lines.push(format!("DROP COLUMN {name}")); + actions.push(format!("DROP COLUMN {name}")); } TableChange::AlterColumn(AlterColumn { column_id, @@ -287,7 +288,7 @@ impl SqlRenderer for PostgresFlavour { columns, changes, &mut before_statements, - &mut lines, + &mut actions, &mut after_statements, self, ); @@ -296,22 +297,23 @@ impl SqlRenderer for PostgresFlavour { let columns = schemas.walk(*column_id); let name = self.quote(columns.previous.name()); - lines.push(format!("DROP COLUMN {name}")); + actions.push(format!("DROP COLUMN {name}")); let col_sql = self.render_column(columns.next); - lines.push(format!("ADD COLUMN {col_sql}")); + actions.push(format!("ADD COLUMN {col_sql}")); } }; } - if lines.is_empty() { + if actions.is_empty() && stats.is_empty() { return Vec::new(); } if self.is_cockroachdb() { - let mut out = Vec::with_capacity(before_statements.len() + after_statements.len() + lines.len()); + let mut out = + Vec::with_capacity(before_statements.len() + after_statements.len() + actions.len() + stats.len()); out.extend(before_statements); - for line in lines { + for line in stats.into_iter().chain(actions) { out.push(format!( "ALTER TABLE {} {}", QuotedWithPrefix::pg_from_table_walker(tables.previous), @@ -321,15 +323,21 @@ impl SqlRenderer for PostgresFlavour { out.extend(after_statements); out } else { - let alter_table = format!( - "ALTER TABLE {} {}", - QuotedWithPrefix::pg_new(tables.previous.namespace(), tables.previous.name()), - lines.join(",\n") - ); - + let alter_tables = if actions.is_empty() { + either::Right(stats.into_iter()) + } else { + either::Left(stats.into_iter().chain(std::iter::once(actions.join(",\n")))) + } + .map(|s| { + format!( + "ALTER TABLE {} {}", + QuotedWithPrefix::pg_new(tables.previous.namespace(), tables.previous.name()), + s + ) + }); before_statements .into_iter() - .chain(std::iter::once(alter_table)) + .chain(alter_tables) .chain(after_statements) .collect() } diff --git a/schema-engine/sql-migration-tests/tests/migrations/postgres.rs b/schema-engine/sql-migration-tests/tests/migrations/postgres.rs index 7aa222fc64bf..42f83b3b26c8 100644 --- a/schema-engine/sql-migration-tests/tests/migrations/postgres.rs +++ b/schema-engine/sql-migration-tests/tests/migrations/postgres.rs @@ -773,3 +773,53 @@ fn dbgenerated_on_generated_unsupported_columns_is_idempotent(api: TestApi) { api.schema_push(schema).send().assert_green().assert_no_steps(); } + +// https://github.com/prisma/prisma/issues/24331 +#[test_connector(tags(Postgres))] +fn remove_map_from_id(api: TestApi) { + let schema_1 = r#" + datasource db { + provider = "postgresql" + url = env("DATABASE_URL") + } + + model Fruits { + id Int @id(map:"custom_name") + name String @db.VarChar + } + "#; + + let schema_2 = r#" + datasource db { + provider = "postgresql" + url = env("DATABASE_URL") + } + + model Fruits { + id Int @id() + name String + } + "#; + + let migration = api.connector_diff( + DiffTarget::Datamodel(vec![("schema.prisma".to_string(), SourceFile::new_static(schema_1))]), + DiffTarget::Datamodel(vec![("schema.prisma".to_string(), SourceFile::new_static(schema_2))]), + None, + ); + + let expected_migration = expect![[r#" + -- AlterTable + ALTER TABLE "Fruits" RENAME CONSTRAINT "custom_name" TO "Fruits_pkey"; + ALTER TABLE "Fruits" ALTER COLUMN "name" SET DATA TYPE TEXT; + "#]]; + + expected_migration.assert_eq(&migration); + + api.schema_push(schema_1).send().assert_green(); + api.schema_push(schema_1).send().assert_green().assert_no_steps(); + api.schema_push(schema_2) + .send() + .assert_green() + .assert_has_executed_steps(); + api.schema_push(schema_2).send().assert_green().assert_no_steps(); +}