Skip to content

Commit

Permalink
migration-engine: fix a bunch of multiSchema migrations (#3378)
Browse files Browse the repository at this point in the history
* migration-engine: fix rename index with multiSchema
* migration-engine: fix dropping enums with multiSchema
* migration-engine: fix drop foreign key for multiSchema
* migration-engine: fix drop index with multiSchema
* migration-engine: fix dropping views when resetting
* migration-engine: fix altering enums with multiSchema
  • Loading branch information
eviefp committed Nov 12, 2022
1 parent a93b663 commit 1545b76
Show file tree
Hide file tree
Showing 10 changed files with 424 additions and 27 deletions.
4 changes: 2 additions & 2 deletions libs/sql-ddl/src/postgres.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,13 @@ impl Display for Column<'_> {
#[derive(Debug)]
pub struct DropIndex<'a> {
/// The name of the index to be dropped.
pub index_name: Cow<'a, str>,
pub index_name: PostgresIdentifier<'a>,
}

impl Display for DropIndex<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("DROP INDEX ")?;
Display::fmt(&PostgresIdentifier::from(self.index_name.as_ref()), f)
write!(f, "{}", self.index_name)
}
}

Expand Down
8 changes: 8 additions & 0 deletions libs/sql-schema-describer/src/walkers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ impl<'a> ViewWalker<'a> {
self.view().definition.as_deref()
}

/// The namespace of the view
pub fn namespace(self) -> Option<&'a str> {
self.schema
.namespaces
.get(self.view().namespace_id.0 as usize)
.map(|s| s.as_str())
}

fn view(self) -> &'a View {
&self.schema.views[self.id.0 as usize]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,13 @@ impl SqlRenderer for PostgresFlavour {
fn render_rename_index(&self, indexes: Pair<IndexWalker<'_>>) -> Vec<String> {
render_step(&mut |step| {
step.render_statement(&mut |stmt| {
let previous_table = indexes.previous.table();
let index_previous_name = match previous_table.namespace() {
Some(ns) => format!("{}.{}", self.quote(ns), self.quote(indexes.previous.name())),
None => self.quote(indexes.previous.name()).to_string(),
};
stmt.push_str("ALTER INDEX ");
stmt.push_display(&Quoted::postgres_ident(indexes.previous.name()));
stmt.push_str(&index_previous_name);
stmt.push_str(" RENAME TO ");
stmt.push_display(&Quoted::postgres_ident(indexes.next.name()));
})
Expand Down Expand Up @@ -428,7 +433,10 @@ impl SqlRenderer for PostgresFlavour {
render_step(&mut |step| {
step.render_statement(&mut |stmt| {
stmt.push_display(&ddl::DropType {
type_name: dropped_enum.name().into(),
type_name: match dropped_enum.namespace() {
Some(ns) => PostgresIdentifier::WithSchema(ns.into(), dropped_enum.name().into()),
None => dropped_enum.name().into(),
},
})
})
})
Expand All @@ -437,14 +445,20 @@ impl SqlRenderer for PostgresFlavour {
fn render_drop_foreign_key(&self, foreign_key: ForeignKeyWalker<'_>) -> String {
format!(
"ALTER TABLE {table} DROP CONSTRAINT {constraint_name}",
table = self.quote(foreign_key.table().name()),
table = match foreign_key.table().namespace() {
Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), foreign_key.table().name().into()),
None => foreign_key.table().name().into(),
},
constraint_name = Quoted::postgres_ident(foreign_key.constraint_name().unwrap()),
)
}

fn render_drop_index(&self, index: IndexWalker<'_>) -> String {
ddl::DropIndex {
index_name: index.name().into(),
index_name: match index.table().namespace() {
Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), index.name().into()),
None => index.name().into(),
},
}
.to_string()
}
Expand All @@ -465,7 +479,10 @@ impl SqlRenderer for PostgresFlavour {

fn render_drop_view(&self, view: ViewWalker<'_>) -> String {
ddl::DropView {
view_name: view.name().into(),
view_name: match view.namespace() {
Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), view.name().into()),
None => view.name().into(),
},
}
.to_string()
}
Expand All @@ -478,6 +495,8 @@ impl SqlRenderer for PostgresFlavour {
let temporary_table_name = format!("_prisma_new_{}", &tables.next.name());
result.push(self.render_create_table_as(
tables.next,
// TODO(MultiSchema): This only matters for CockroachDB, which is not currently
// supported by MultiSchema.
TableName(None, Quoted::postgres_ident(&temporary_table_name)),
));

Expand Down Expand Up @@ -896,7 +915,13 @@ fn render_postgres_alter_enum(
.map(|created_value| {
format!(
"ALTER TYPE {enum_name} ADD VALUE {value}",
enum_name = Quoted::postgres_ident(schemas.walk(alter_enum.id).previous.name()),
enum_name = match schemas.walk(alter_enum.id).previous.namespace() {
Some(namespace) => PostgresIdentifier::WithSchema(
namespace.into(),
schemas.walk(alter_enum.id).previous.name().into()
),
None => schemas.walk(alter_enum.id).previous.name().into(),
},
value = Quoted::postgres_string(created_value)
)
})
Expand Down Expand Up @@ -1002,7 +1027,10 @@ fn render_postgres_alter_enum(
// Drop old enum
{
let sql = ddl::DropType {
type_name: tmp_old_name.as_str().into(),
type_name: match enums.previous.namespace() {
Some(ns) => PostgresIdentifier::WithSchema(ns.into(), tmp_old_name.as_str().into()),
None => tmp_old_name.as_str().into(),
},
}
.to_string();

Expand Down
14 changes: 8 additions & 6 deletions migration-engine/migration-engine-tests/src/commands/reset.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use migration_core::{migration_connector::MigrationConnector, CoreResult};
use migration_core::{
migration_connector::{MigrationConnector, Namespaces},
CoreResult,
};

#[must_use = "This struct does nothing on its own. See Reset::send()"]
pub struct Reset<'a> {
Expand All @@ -16,17 +19,16 @@ impl<'a> Reset<'a> {
self
}

pub async fn send(self) -> CoreResult<ResetAssertion> {
// TODO(MultiSchema): should we somehow send Namespaces here?
self.api.reset(self.soft, None).await?;
pub async fn send(self, namespaces: Option<Namespaces>) -> CoreResult<ResetAssertion> {
self.api.reset(self.soft, namespaces).await?;

Ok(ResetAssertion {})
}

/// Execute the command and expect it to succeed.
#[track_caller]
pub fn send_sync(self) -> ResetAssertion {
test_setup::runtime::run_with_thread_local_runtime(self.send()).unwrap()
pub fn send_sync(self, namespaces: Option<Namespaces>) -> ResetAssertion {
test_setup::runtime::run_with_thread_local_runtime(self.send(namespaces)).unwrap()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn soft_resets_work_on_cockroachdb(mut api: TestApi) {

api.raw_cmd(initial);
api.assert_schema().assert_tables_count(1).assert_has_table("Cat");
api.reset().soft(true).send_sync();
api.reset().soft(true).send_sync(None);
api.assert_schema().assert_tables_count(0);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ fn alter_enum_and_change_default_must_work(api: TestApi) {
});

// we repeat the same tests with migrations, so we can observe the generated SQL statements.
api.reset().send_sync();
api.reset().send_sync(None);
api.assert_schema().assert_tables_count(0);

let dir = api.create_migrations_directory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn reset_clears_udts(api: TestApi) {
);
assert_eq!(1, schemas.len());

api.reset().send_sync();
api.reset().send_sync(None);

let schemas = api.query_raw(
&format!(
Expand Down

0 comments on commit 1545b76

Please sign in to comment.