migration-engine: improve multiSchema related code in renderer (#3439)

libs/sql-ddl/src/postgres.rs

@@ -257,6 +257,15 @@ pub enum PostgresIdentifier<'a> {
     WithSchema(Cow<'a, str>, Cow<'a, str>),
 }
 
+impl<'a> PostgresIdentifier<'a> {
+    pub fn new(namespace: Option<&'a str>, name: &'a str) -> Self {
+        match namespace {
+            Some(ns) => PostgresIdentifier::WithSchema(ns.into(), name.into()),
+            None => PostgresIdentifier::Simple(name.into()),
+        }
+    }
+}
+
 impl Default for PostgresIdentifier<'_> {
     fn default() -> Self {
         PostgresIdentifier::Simple(Cow::Borrowed(""))

migration-engine/connectors/sql-migration-connector/src/sql_renderer.rs

@@ -16,7 +16,7 @@ mod sqlite_renderer;
 
 pub(crate) use common::IteratorJoin;
 
-use self::common::{Quoted, TableName};
+use self::common::{Quoted, QuotedWithPrefix};
 use crate::{
     pair::Pair,
     sql_migration::{
@@ -63,7 +63,7 @@ pub(crate) trait SqlRenderer {
     fn render_create_table(&self, table: TableWalker<'_>) -> String;
 
     /// Render a table creation with the provided table name.
-    fn render_create_table_as(&self, table: TableWalker<'_>, table_name: TableName<&str>) -> String;
+    fn render_create_table_as(&self, table: TableWalker<'_>, table_name: QuotedWithPrefix<&str>) -> String;
 
     fn render_drop_and_recreate_index(&self, _indexes: Pair<IndexWalker<'_>>) -> Vec<String> {
         unreachable!("unreachable render_drop_and_recreate_index")

migration-engine/connectors/sql-migration-connector/src/sql_renderer/common.rs

@@ -3,16 +3,23 @@ use std::fmt::{Display, Write as _};
 
 pub(super) const SQL_INDENTATION: &str = "    ";
 
-/// A table name with an optional schema prefix.
-pub(crate) struct TableName<T>(pub(crate) Option<Quoted<T>>, pub(crate) Quoted<T>);
+/// A quoted identifier with an optional schema prefix.
+pub(crate) struct QuotedWithPrefix<T>(pub(crate) Option<Quoted<T>>, pub(crate) Quoted<T>);
 
-impl<T> TableName<T> {
-    pub(crate) fn new(namespace: Option<Quoted<T>>, name: Quoted<T>) -> Self {
-        TableName(namespace, name)
+impl QuotedWithPrefix<&str> {
+    pub(crate) fn pg_new<'a>(namespace: Option<&'a str>, name: &'a str) -> QuotedWithPrefix<&'a str> {
+        QuotedWithPrefix(namespace.map(Quoted::postgres_ident), Quoted::postgres_ident(name))
+    }
+
+    pub(crate) fn pg_from_table_walker(table: TableWalker<'_>) -> QuotedWithPrefix<&str> {
+        QuotedWithPrefix(
+            table.namespace().map(Quoted::postgres_ident),
+            Quoted::postgres_ident(table.name()),
+        )
     }
 }
 
-impl<T> Display for TableName<T>
+impl<T> Display for QuotedWithPrefix<T>
 where
     T: Display,
 {

migration-engine/connectors/sql-migration-connector/src/sql_renderer/mssql_renderer.rs

@@ -13,17 +13,17 @@ use sql_schema_describer::{self as sql, mssql::MssqlSchemaExt};
 use std::{borrow::Cow, fmt::Write};
 
 impl MssqlFlavour {
-    fn table_name<'a>(&'a self, table: sql::TableWalker<'a>) -> TableName<&'a str> {
-        TableName(
+    fn table_name<'a>(&'a self, table: sql::TableWalker<'a>) -> QuotedWithPrefix<&'a str> {
+        QuotedWithPrefix(
             Some(Quoted::mssql_ident(
                 table.namespace().unwrap_or_else(|| self.schema_name()),
             )),
             Quoted::mssql_ident(table.name()),
         )
     }
 
-    fn quote_with_schema<'a>(&'a self, name: &'a str) -> TableName<&'a str> {
-        TableName(Some(Quoted::mssql_ident(self.schema_name())), Quoted::mssql_ident(name))
+    fn quote_with_schema<'a>(&'a self, name: &'a str) -> QuotedWithPrefix<&'a str> {
+        QuotedWithPrefix(Some(Quoted::mssql_ident(self.schema_name())), Quoted::mssql_ident(name))
     }
 
     fn render_column(&self, column: sql::ColumnWalker<'_>) -> String {
@@ -149,7 +149,7 @@ impl SqlRenderer for MssqlFlavour {
         self.render_create_table_as(table, self.table_name(table))
     }
 
-    fn render_create_table_as(&self, table: sql::TableWalker<'_>, table_name: TableName<&str>) -> String {
+    fn render_create_table_as(&self, table: sql::TableWalker<'_>, table_name: QuotedWithPrefix<&str>) -> String {
         let columns: String = table.columns().map(|column| self.render_column(column)).join(",\n    ");
         let mssql_schema_ext: &MssqlSchemaExt = table.schema.downcast_connector_data();
 

migration-engine/connectors/sql-migration-connector/src/sql_renderer/mysql_renderer.rs

@@ -220,7 +220,7 @@ impl SqlRenderer for MysqlFlavour {
         .to_string()
     }
 
-    fn render_create_table_as(&self, table: TableWalker<'_>, table_name: TableName<&str>) -> String {
+    fn render_create_table_as(&self, table: TableWalker<'_>, table_name: QuotedWithPrefix<&str>) -> String {
         ddl::CreateTable {
             table_name: &table_name,
             columns: table.columns().map(|col| self.render_column(col)).collect(),
@@ -328,7 +328,7 @@ impl SqlRenderer for MysqlFlavour {
     }
 
     fn render_create_table(&self, table: TableWalker<'_>) -> String {
-        self.render_create_table_as(table, TableName(None, Quoted::mysql_ident(table.name())))
+        self.render_create_table_as(table, QuotedWithPrefix(None, Quoted::mysql_ident(table.name())))
     }
 
     fn render_drop_view(&self, view: ViewWalker<'_>) -> String {

migration-engine/connectors/sql-migration-connector/src/sql_renderer/postgres_renderer.rs

@@ -155,18 +155,12 @@ impl SqlRenderer for PostgresFlavour {
 
     fn render_add_foreign_key(&self, foreign_key: ForeignKeyWalker<'_>) -> String {
         ddl::AlterTable {
-            table_name: &TableName(
-                foreign_key.table().namespace().map(Quoted::postgres_ident),
-                Quoted::postgres_ident(foreign_key.table().name()),
-            ),
+            table_name: &QuotedWithPrefix::pg_from_table_walker(foreign_key.table()),
             clauses: vec![ddl::AlterTableClause::AddForeignKey(ddl::ForeignKey {
                 constrained_columns: foreign_key.constrained_columns().map(|c| c.name().into()).collect(),
                 referenced_columns: foreign_key.referenced_columns().map(|c| c.name().into()).collect(),
                 constraint_name: foreign_key.constraint_name().map(From::from),
-                referenced_table: &TableName(
-                    foreign_key.referenced_table().namespace().map(Quoted::postgres_ident),
-                    Quoted::postgres_ident(foreign_key.referenced_table().name()),
-                ),
+                referenced_table: &QuotedWithPrefix::pg_from_table_walker(foreign_key.referenced_table()),
                 on_delete: Some(match foreign_key.on_delete_action() {
                     ForeignKeyAction::Cascade => ddl::ForeignKeyAction::Cascade,
                     ForeignKeyAction::NoAction => ddl::ForeignKeyAction::NoAction,
@@ -225,12 +219,9 @@ impl SqlRenderer for PostgresFlavour {
         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(),
-                };
+                let index_previous_name = QuotedWithPrefix::pg_new(previous_table.namespace(), indexes.previous.name());
                 stmt.push_str("ALTER INDEX ");
-                stmt.push_str(&index_previous_name);
+                stmt.push_str(&index_previous_name.to_string());
                 stmt.push_str(" RENAME TO ");
                 // Postgres assumes we use the same schema as the previous name's, so we're not
                 // allowed to qualify this identifier.
@@ -331,10 +322,7 @@ impl SqlRenderer for PostgresFlavour {
         } else {
             let alter_table = format!(
                 "ALTER TABLE {} {}",
-                TableName::new(
-                    tables.previous.namespace().map(|ns| self.quote(ns)),
-                    self.quote(tables.previous.name())
-                ),
+                QuotedWithPrefix::pg_new(tables.previous.namespace(), tables.previous.name()),
                 lines.join(",\n")
             );
 
@@ -350,10 +338,7 @@ impl SqlRenderer for PostgresFlavour {
         render_step(&mut |step| {
             step.render_statement(&mut |stmt| {
                 stmt.push_str("CREATE TYPE ");
-                stmt.push_display(&TableName(
-                    enm.namespace().map(Quoted::postgres_ident),
-                    Quoted::postgres_ident(enm.name()),
-                ));
+                stmt.push_display(&QuotedWithPrefix::pg_new(enm.namespace(), enm.name()));
                 stmt.push_str(" AS ENUM (");
                 let mut values = enm.values().peekable();
                 while let Some(value) = values.next() {
@@ -373,10 +358,7 @@ impl SqlRenderer for PostgresFlavour {
         ddl::CreateIndex {
             index_name: index.name().into(),
             is_unique: index.is_unique(),
-            table_reference: &TableName(
-                index.table().namespace().map(Quoted::postgres_ident),
-                Quoted::postgres_ident(index.table().name()),
-            ),
+            table_reference: &QuotedWithPrefix::pg_from_table_walker(index.table()),
             using: Some(match pg_ext.index_algorithm(index.id) {
                 SqlIndexAlgorithm::BTree => ddl::IndexAlgorithm::BTree,
                 SqlIndexAlgorithm::Hash => ddl::IndexAlgorithm::Hash,
@@ -406,16 +388,10 @@ impl SqlRenderer for PostgresFlavour {
     }
 
     fn render_create_table(&self, table: TableWalker<'_>) -> String {
-        self.render_create_table_as(
-            table,
-            TableName(
-                table.namespace().map(Quoted::postgres_ident),
-                Quoted::postgres_ident(table.name()),
-            ),
-        )
+        self.render_create_table_as(table, QuotedWithPrefix::pg_from_table_walker(table))
     }
 
-    fn render_create_table_as(&self, table: TableWalker<'_>, table_name: TableName<&str>) -> String {
+    fn render_create_table_as(&self, table: TableWalker<'_>, table_name: QuotedWithPrefix<&str>) -> String {
         let columns: String = table.columns().map(|column| self.render_column(column)).join(",\n");
 
         let pk = if let Some(pk) = table.primary_key() {
@@ -438,10 +414,7 @@ impl SqlRenderer for PostgresFlavour {
         render_step(&mut |step| {
             step.render_statement(&mut |stmt| {
                 stmt.push_display(&ddl::DropType {
-                    type_name: match dropped_enum.namespace() {
-                        Some(ns) => PostgresIdentifier::WithSchema(ns.into(), dropped_enum.name().into()),
-                        None => dropped_enum.name().into(),
-                    },
+                    type_name: PostgresIdentifier::new(dropped_enum.namespace(), dropped_enum.name()),
                 })
             })
         })
@@ -450,20 +423,14 @@ impl SqlRenderer for PostgresFlavour {
     fn render_drop_foreign_key(&self, foreign_key: ForeignKeyWalker<'_>) -> String {
         format!(
             "ALTER TABLE {table} DROP CONSTRAINT {constraint_name}",
-            table = match foreign_key.table().namespace() {
-                Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), foreign_key.table().name().into()),
-                None => foreign_key.table().name().into(),
-            },
+            table = PostgresIdentifier::new(foreign_key.table().namespace(), foreign_key.table().name()),
             constraint_name = Quoted::postgres_ident(foreign_key.constraint_name().unwrap()),
         )
     }
 
     fn render_drop_index(&self, index: IndexWalker<'_>) -> String {
         ddl::DropIndex {
-            index_name: match index.table().namespace() {
-                Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), index.name().into()),
-                None => index.name().into(),
-            },
+            index_name: PostgresIdentifier::new(index.table().namespace(), index.name()),
         }
         .to_string()
     }
@@ -472,10 +439,7 @@ impl SqlRenderer for PostgresFlavour {
         render_step(&mut |step| {
             step.render_statement(&mut |stmt| {
                 stmt.push_display(&ddl::DropTable {
-                    table_name: match namespace {
-                        Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), table_name.into()),
-                        None => table_name.into(),
-                    },
+                    table_name: PostgresIdentifier::new(namespace, table_name),
                     cascade: false,
                 })
             })
@@ -484,10 +448,7 @@ impl SqlRenderer for PostgresFlavour {
 
     fn render_drop_view(&self, view: ViewWalker<'_>) -> String {
         ddl::DropView {
-            view_name: match view.namespace() {
-                Some(namespace) => PostgresIdentifier::WithSchema(namespace.into(), view.name().into()),
-                None => view.name().into(),
-            },
+            view_name: PostgresIdentifier::new(view.namespace(), view.name()),
         }
         .to_string()
     }
@@ -502,7 +463,7 @@ impl SqlRenderer for PostgresFlavour {
                 tables.next,
                 // TODO(MultiSchema): This only matters for CockroachDB, which is not currently
                 // supported by MultiSchema.
-                TableName(None, Quoted::postgres_ident(&temporary_table_name)),
+                QuotedWithPrefix(None, Quoted::postgres_ident(&temporary_table_name)),
             ));
 
             let columns: Vec<_> = redefine_table
@@ -563,10 +524,7 @@ impl SqlRenderer for PostgresFlavour {
     fn render_rename_foreign_key(&self, fks: Pair<ForeignKeyWalker<'_>>) -> String {
         format!(
             r#"ALTER TABLE {table} RENAME CONSTRAINT {previous} TO {next}"#,
-            table = TableName(
-                fks.previous.table().namespace().map(Quoted::postgres_ident),
-                Quoted::postgres_ident(fks.previous.table().name()),
-            ),
+            table = QuotedWithPrefix::pg_from_table_walker(fks.previous.table()),
             previous = self.quote(fks.previous.constraint_name().unwrap()),
             next = self.quote(fks.next.constraint_name().unwrap()),
         )
@@ -576,10 +534,7 @@ impl SqlRenderer for PostgresFlavour {
 fn render_column_type(col: ColumnWalker<'_>, flavour: &PostgresFlavour) -> Cow<'static, str> {
     let t = col.column_type();
     if let Some(enm) = col.column_type_family_as_enum() {
-        let name = TableName(
-            enm.namespace().map(Quoted::postgres_ident),
-            Quoted::postgres_ident(enm.name()),
-        );
+        let name = QuotedWithPrefix::pg_new(enm.namespace(), enm.name());
         let arity = if t.arity.is_list() { "[]" } else { "" };
         return format!("{name}{arity}").into();
     }
@@ -733,10 +688,7 @@ fn render_alter_column(
     flavour: &PostgresFlavour,
 ) {
     let steps = expand_alter_column(columns, column_changes);
-    let table_name = TableName::new(
-        columns.previous.table().namespace().map(Quoted::postgres_ident),
-        Quoted::postgres_ident(columns.previous.table().name()),
-    );
+    let table_name = QuotedWithPrefix::pg_from_table_walker(columns.previous.table());
     let column_name = Quoted::postgres_ident(columns.previous.name());
 
     let alter_column_prefix = format!("ALTER COLUMN {}", column_name);
@@ -930,13 +882,10 @@ fn render_postgres_alter_enum(
             .map(|created_value| {
                 format!(
                     "ALTER TYPE {enum_name} ADD VALUE {value}",
-                    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(),
-                    },
+                    enum_name = QuotedWithPrefix::pg_new(
+                        schemas.walk(alter_enum.id).previous.namespace(),
+                        schemas.walk(alter_enum.id).previous.name()
+                    ),
                     value = Quoted::postgres_string(created_value)
                 )
             })
@@ -964,10 +913,7 @@ fn render_postgres_alter_enum(
     let mut stmts = Vec::with_capacity(10);
 
     let temporary_enum_name = format!("{}_new", &enums.next.name());
-    let tmp_name = TableName::new(
-        enums.next.namespace().map(Quoted::postgres_ident),
-        Quoted::postgres_ident(temporary_enum_name.as_str()),
-    );
+    let tmp_name = QuotedWithPrefix::pg_new(enums.next.namespace(), temporary_enum_name.as_str());
     let tmp_old_name = format!("{}_old", &enums.previous.name());
 
     stmts.push("BEGIN".to_string());
@@ -990,10 +936,7 @@ fn render_postgres_alter_enum(
 
             let drop_default = format!(
                 r#"ALTER TABLE {table_name} ALTER COLUMN {column_name} DROP DEFAULT"#,
-                table_name = TableName::new(
-                    column.table().namespace().map(Quoted::postgres_ident),
-                    Quoted::postgres_ident(column.table().name())
-                ),
+                table_name = QuotedWithPrefix::pg_from_table_walker(column.table()),
                 column_name = Quoted::postgres_ident(column.name()),
             );
 
@@ -1014,10 +957,7 @@ fn render_postgres_alter_enum(
                 "ALTER TABLE {table_name} \
                             ALTER COLUMN {column_name} TYPE {tmp_name}{array} \
                                 USING ({column_name}::text::{tmp_name}{array})",
-                table_name = TableName::new(
-                    column.table().namespace().map(Quoted::postgres_ident),
-                    Quoted::postgres_ident(column.table().name())
-                ),
+                table_name = QuotedWithPrefix::pg_from_table_walker(column.table()),
                 column_name = Quoted::postgres_ident(column.name()),
                 array = array,
             );
@@ -1030,10 +970,7 @@ fn render_postgres_alter_enum(
     {
         let sql = format!(
             "ALTER TYPE {enum_name} RENAME TO {tmp_old_name}",
-            enum_name = TableName::new(
-                enums.previous.namespace().map(Quoted::postgres_ident),
-                Quoted::postgres_ident(enums.previous.name())
-            ),
+            enum_name = QuotedWithPrefix::pg_new(enums.previous.namespace(), enums.previous.name()),
             tmp_old_name = Quoted::postgres_ident(&tmp_old_name)
         );
 
@@ -1053,10 +990,7 @@ fn render_postgres_alter_enum(
     // Drop old enum
     {
         let sql = ddl::DropType {
-            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(),
-            },
+            type_name: PostgresIdentifier::new(enums.previous.namespace(), tmp_old_name.as_str()),
         }
         .to_string();
 
@@ -1078,10 +1012,7 @@ fn render_postgres_alter_enum(
 
             let set_default = format!(
                 "ALTER TABLE {table_name} ALTER COLUMN {column_name} SET DEFAULT {default}",
-                table_name = TableName::new(
-                    columns.previous.table().namespace().map(Quoted::postgres_ident),
-                    Quoted::postgres_ident(table_name)
-                ),
+                table_name = QuotedWithPrefix::pg_new(columns.previous.table().namespace(), table_name),
                 column_name = Quoted::postgres_ident(&column_name),
                 default = default_str,
             );