Skip to content

Commit

Permalink
feat(se): allow model/table renaming in postgres and sqlserver
Browse files Browse the repository at this point in the history
  • Loading branch information
eruditmorina committed Jun 10, 2024
1 parent 9ee8c02 commit c27b7da
Show file tree
Hide file tree
Showing 14 changed files with 203 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ impl SqlSchemaConnector {
}
TableChange::AddPrimaryKey { .. } => (),
TableChange::RenamePrimaryKey { .. } => (),
TableChange::RenameTo { .. } => (),
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,13 @@ impl SqlMigration {
out.push_str(")\n");
out.push_str(")\n");
}
TableChange::RenameTo => {
out.push_str(" [+] Renamed table `");
out.push_str(tables.previous.name());
out.push_str("` to `");
out.push_str(tables.next.name());
out.push_str("`\n")
}
}
}
}
Expand Down Expand Up @@ -580,6 +587,7 @@ pub(crate) enum TableChange {
DropPrimaryKey,
AddPrimaryKey,
RenamePrimaryKey,
RenameTo,
}

#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub(crate) fn create_statements(
add_columns: Vec::new(),
drop_columns: Vec::new(),
column_mods: Vec::new(),
rename_table: false,
};

constructor.into_statements()
Expand All @@ -49,12 +50,16 @@ struct AlterTableConstructor<'a> {
add_columns: Vec<String>,
drop_columns: Vec<String>,
column_mods: Vec<String>,
rename_table: bool,
}

impl<'a> AlterTableConstructor<'a> {
fn into_statements(mut self) -> Vec<String> {
for change in self.changes {
match change {
TableChange::RenameTo => {
self.rename_table = true;
}
TableChange::DropPrimaryKey => {
self.drop_primary_key();
}
Expand Down Expand Up @@ -142,6 +147,23 @@ impl<'a> AlterTableConstructor<'a> {
));
}

if self.rename_table {
let with_schema = format!(
"{}.{}",
self.tables
.previous
.namespace()
.unwrap_or_else(|| self.renderer.schema_name()),
self.tables.previous.name()
);

statements.push(format!(
"EXEC SP_RENAME N{}, N{}",
Quoted::mssql_string(with_schema),
Quoted::mssql_string(self.tables.next.name()),
));
}

statements
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ impl SqlRenderer for MysqlFlavour {

for change in changes {
match change {
TableChange::RenameTo => unreachable!("No Renaming Tables on Mysql"),
TableChange::DropPrimaryKey => lines.push(sql_ddl::mysql::AlterTableClause::DropPrimaryKey.to_string()),
TableChange::RenamePrimaryKey => unreachable!("No Renaming Primary Keys on Mysql"),
TableChange::AddPrimaryKey => lines.push(format!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,14 @@ 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 separate_lines = 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::RenameTo => separate_lines.push(format!("RENAME TO {}", self.quote(tables.next.name()))),
TableChange::DropPrimaryKey => lines.push(format!(
"DROP CONSTRAINT {}",
Quoted::postgres_ident(tables.previous.primary_key().unwrap().name())
Expand Down Expand Up @@ -304,14 +306,14 @@ impl SqlRenderer for PostgresFlavour {
};
}

if lines.is_empty() {
if lines.is_empty() && separate_lines.is_empty() {
return Vec::new();
}

if self.is_cockroachdb() {
let mut out = Vec::with_capacity(before_statements.len() + after_statements.len() + lines.len());
out.extend(before_statements);
for line in lines {
for line in lines.into_iter().chain(separate_lines) {
out.push(format!(
"ALTER TABLE {} {}",
QuotedWithPrefix::pg_from_table_walker(tables.previous),
Expand All @@ -321,12 +323,16 @@ 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 table = QuotedWithPrefix::pg_new(tables.previous.namespace(), tables.previous.name());
for line in separate_lines {
after_statements.push(format!("ALTER TABLE {} {}", table, line))
}

if lines.is_empty() {
return before_statements.into_iter().chain(after_statements).collect();
}

let alter_table = format!("ALTER TABLE {} {}", table, lines.join(",\n"));
before_statements
.into_iter()
.chain(std::iter::once(alter_table))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ impl SqlRenderer for SqliteFlavour {
TableChange::DropColumn { .. } => unreachable!("DropColumn on SQLite"),
TableChange::DropPrimaryKey { .. } => unreachable!("DropPrimaryKey on SQLite"),
TableChange::RenamePrimaryKey { .. } => unreachable!("AddPrimaryKey on SQLite"),
TableChange::RenameTo => unreachable!("RenameTo on SQLite"),
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ fn push_altered_table_steps(steps: &mut Vec<SqlMigrationStep>, db: &DifferDataba

// Order matters.
let mut changes = Vec::new();
renamed_table(&table, &mut changes);

if let Some(change) = dropped_primary_key(&table) {
changes.push(change)
}
Expand Down Expand Up @@ -305,6 +307,12 @@ fn renamed_primary_key(differ: &TableDiffer<'_, '_>) -> Option<TableChange> {
.map(|_| TableChange::RenamePrimaryKey)
}

fn renamed_table(differ: &TableDiffer<'_, '_>, changes: &mut Vec<TableChange>) {
if differ.tables.previous.is_renamed_table(differ.tables.next) {
changes.push(TableChange::RenameTo)
}
}

fn push_alter_primary_key(differ: &TableDiffer<'_, '_>, steps: &mut Vec<SqlMigrationStep>) {
if !differ.db.flavour.can_alter_primary_keys() {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,22 @@ impl<'a> DifferDatabase<'a> {
} else {
Cow::Borrowed(table.name())
};
db.tables.insert(
(table.namespace().map(Cow::Borrowed), table_name),
MigrationPair::new(Some(table.id), None),
);

// Check if a table with similar attributes but different name exists in the next schema.
let similar_table_exists_in_next = schemas
.next
.describer_schema
.table_walkers()
.any(|t| !table_is_ignored(t.name()) && t.is_renamed_table(table));

// Prevent adding table renames in diff twice.
// Insert renames only when iterating the next schema, and only if database supports it
if !flavour.can_rename_tables() || (flavour.can_rename_tables() && !similar_table_exists_in_next) {
db.tables.insert(
(table.namespace().map(Cow::Borrowed), table_name),
MigrationPair::new(Some(table.id), None),
);
}
}

// Then insert all tables from the next schema. Since we have all the
Expand All @@ -118,6 +130,19 @@ impl<'a> DifferDatabase<'a> {
.tables
.entry((table.namespace().map(Cow::Borrowed), table_name))
.or_default();

// Check if a table with similar attributes but different name exists in the previous schema.
let similar_table_in_previous = schemas
.previous
.describer_schema
.table_walkers()
.find(|t| !table_is_ignored(t.name()) && t.is_renamed_table(table));

if let Some(previous_table) = similar_table_in_previous {
if flavour.can_rename_tables() {
entry.previous = Some(previous_table.id);
}
};
entry.next = Some(table.id);

// Deal with tables that are both in the previous and the next
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ pub(crate) trait SqlSchemaDifferFlavour {
false
}

// Returns `true` if the database supports names for primary key constraints
fn can_rename_tables(&self) -> bool {
false
}

/// If this returns `true`, the differ will generate
/// SqlMigrationStep::RedefineIndex steps instead of
/// SqlMigrationStep::AlterIndex.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ use psl::builtin_connectors::{MsSqlType, MsSqlTypeParameter};
use sql_schema_describer::{self as sql, mssql::MssqlSchemaExt, ColumnTypeFamily, TableColumnId};

impl SqlSchemaDifferFlavour for MssqlFlavour {
fn can_rename_tables(&self) -> bool {
true
}

fn can_rename_foreign_key(&self) -> bool {
true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ impl SqlSchemaDifferFlavour for PostgresFlavour {
self.is_cockroachdb()
}

fn can_rename_tables(&self) -> bool {
true
}

fn can_rename_foreign_key(&self) -> bool {
true
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1312,3 +1312,70 @@ fn alter_constraint_name(mut api: TestApi) {
migration.expect_contents(expected_script)
});
}

#[test_connector(exclude(Mysql, Sqlite))]
fn rename_table_with_explicit_id(mut api: TestApi) {
let dm1 = api.datamodel_with_provider(
r#"
model A {
id Int @id(map: "custom_id")
other Int
}
"#,
);

let dir = api.create_migrations_directory();
api.create_migration("dm1_migration", &dm1, &dir).send_sync();

let custom_dm = api.datamodel_with_provider(
r#"
model B {
id Int @id(map: "custom_id")
other Int
}
"#,
);

let is_mssql = api.is_mssql();
let is_postgres = api.is_postgres();
let is_postgres15 = api.is_postgres_15();
let is_postgres16 = api.is_postgres_16();
let is_cockroach = api.is_cockroach();

api.create_migration("dm2_migration", &custom_dm, &dir)
.send_sync()
.assert_migration_directories_count(2)
.assert_migration("dm2_migration", move |migration| {
let expected_script = if is_mssql {
expect![[r#"
BEGIN TRY
BEGIN TRAN;
-- AlterTable
EXEC SP_RENAME N'dbo.A', N'B';
COMMIT TRAN;
END TRY
BEGIN CATCH
IF @@TRANCOUNT > 0
BEGIN
ROLLBACK TRAN;
END;
THROW
END CATCH
"#]]
} else if is_cockroach || is_postgres || is_postgres15 || is_postgres16 {
expect![[r#"
-- AlterTable
ALTER TABLE "A" RENAME TO "B";
"#]]
} else {
panic!()
};
migration.expect_contents(expected_script)
});
}
32 changes: 32 additions & 0 deletions schema-engine/sql-migration-tests/tests/migrations/basic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,3 +503,35 @@ fn adding_a_primary_key_must_work(api: TestApi) {
api.assert_schema()
.assert_table("Test", |t| t.assert_pk(|pk| pk.assert_constraint_name("Test_pkey")));
}

#[test_connector]
fn renaming_table_must_work(api: TestApi) {
let id = if api.is_sqlite() || api.is_mysql() {
""
} else {
r#"(map: "custom_id")"#
};

let dm1 = format!(
r#"
model A {{
id Int @id{id}
other Int
}}
"#
);
api.schema_push_w_datasource(dm1).send().assert_green();

let dm2 = format!(
r#"
model B {{
id Int @id{id}
other Int
}}
"#
);
api.schema_push_w_datasource(dm2).send().assert_green();

api.assert_schema().assert_has_no_table("A");
api.assert_schema().assert_has_table("B");
}
8 changes: 8 additions & 0 deletions schema-engine/sql-schema-describer/src/walkers/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,14 @@ impl<'a> TableWalker<'a> {
.is_ok()
}

/// Returns whether two tables have same properties, belong to the same table, but have different name.
pub fn is_renamed_table(self, other: TableWalker<'_>) -> bool {
self.name() != other.name()
&& self.table().namespace_id == other.table().namespace_id
&& self.table().properties == other.table().properties
&& self.primary_key().unwrap().name() == other.primary_key().unwrap().name()
}

/// The check constraint names for the table.
pub fn check_constraints(self) -> impl ExactSizeIterator<Item = &'a str> {
let low = self.schema.check_constraints.partition_point(|(id, _)| *id < self.id);
Expand Down

0 comments on commit c27b7da

Please sign in to comment.