Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(se): allow model/table renaming in postgres and sqlserver #4914

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading