Skip to content

Commit

Permalink
fix: alter table postgres miss render non-action
Browse files Browse the repository at this point in the history
  • Loading branch information
zzau13 committed Sep 5, 2024
1 parent f2561ec commit 1bd04d3
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -230,23 +230,24 @@ impl SqlRenderer for PostgresFlavour {

fn render_alter_table(&self, alter_table: &AlterTable, schemas: MigrationPair<&SqlSchema>) -> Vec<String> {
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(),
Expand All @@ -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,
Expand All @@ -287,7 +288,7 @@ impl SqlRenderer for PostgresFlavour {
columns,
changes,
&mut before_statements,
&mut lines,
&mut actions,
&mut after_statements,
self,
);
Expand All @@ -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),
Expand All @@ -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()
}
Expand Down
50 changes: 50 additions & 0 deletions schema-engine/sql-migration-tests/tests/migrations/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit 1bd04d3

Please sign in to comment.