me: never try to migrate a column with @default(dbgenerated())

closes prisma/prisma#14799
prisma · Aug 29, 2022 · 587ec25 · 587ec25
1 parent c875e43
commit 587ec25
Show file tree

Hide file tree

Showing 17 changed files with 124 additions and 37 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/introspection-engine/connectors/sql-introspection-connector/src/defaults.rs b/introspection-engine/connectors/sql-introspection-connector/src/defaults.rs
@@ -60,7 +60,9 @@ pub(crate) fn calculate_default(column: sql::ColumnWalker<'_>, ctx: &mut Context
             column,
         )),
         (Some(sql::DefaultKind::DbGenerated(default_string)), _) => Some(set_default(
-            dml::DefaultValue::new_expression(dml::ValueGenerator::new_dbgenerated(default_string.clone())),
+            dml::DefaultValue::new_expression(dml::ValueGenerator::new_dbgenerated(
+                default_string.as_ref().unwrap().clone(),
+            )),
             column,
         )),
         (Some(sql::DefaultKind::Value(val)), _) => {

diff --git a/libs/sql-schema-describer/src/lib.rs b/libs/sql-schema-describer/src/lib.rs
@@ -584,12 +584,12 @@ pub enum DefaultKind {
     /// A unique row ID,
     UniqueRowid,
     /// An unrecognized Default Value
-    DbGenerated(String),
+    DbGenerated(Option<String>),
 }
 
 impl DefaultValue {
     pub fn db_generated(val: impl Into<String>) -> Self {
-        Self::new(DefaultKind::DbGenerated(val.into()))
+        Self::new(DefaultKind::DbGenerated(Some(val.into())))
     }
 
     pub fn now() -> Self {

diff --git a/libs/sql-schema-describer/src/postgres.rs b/libs/sql-schema-describer/src/postgres.rs
@@ -636,7 +636,7 @@ impl<'a> SqlSchemaDescriber<'a> {
                 || (self.is_cockroach()
                     && matches!(
                         default.as_ref().map(|d| d.kind()),
-                        Some(DefaultKind::DbGenerated(s)) if s == "unique_rowid()"
+                        Some(DefaultKind::DbGenerated(Some(s))) if s == "unique_rowid()"
                     ));
 
             let col = Column {

diff --git a/libs/sql-schema-describer/src/postgres/default.rs b/libs/sql-schema-describer/src/postgres/default.rs
@@ -88,7 +88,7 @@ pub(super) fn get_default_value(default_string: &str, tpe: &ColumnType) -> Optio
 
     Some(match parsed_default {
         Some(default_value) => default_value,
-        None => DefaultValue::db_generated(default_string),
+        None => DefaultValue::db_generated(default_string.to_owned()),
     })
 }
 
@@ -504,7 +504,7 @@ fn get_list_default_value(parser: &mut Parser<'_>, tpe: &ColumnType) -> DefaultV
 
     values
         .map(|values| DefaultValue::value(PrismaValue::List(values)))
-        .unwrap_or_else(|| DefaultValue::db_generated(parser.input))
+        .unwrap_or_else(|| DefaultValue::db_generated(parser.input.to_owned()))
 }
 
 /// Some(()) on valid cast or absence of cast. None if we can't make sense of the input.

diff --git a/libs/sql-schema-describer/tests/describers/mysql_describer_tests.rs b/libs/sql-schema-describer/tests/describers/mysql_describer_tests.rs
@@ -3359,7 +3359,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(abs(8) + abs(8))",
+                                    Some(
+                                        "(abs(8) + abs(8))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3386,7 +3388,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(abs(8))",
+                                    Some(
+                                        "(abs(8))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3413,7 +3417,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(abs(8))",
+                                    Some(
+                                        "(abs(8))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3447,7 +3453,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(abs(8))",
+                                    Some(
+                                        "(abs(8))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3474,7 +3482,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(ifnull(1,0))",
+                                    Some(
+                                        "(ifnull(1,0))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3503,7 +3513,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(left(uuid(),8))",
+                                    Some(
+                                        "(left(uuid(),8))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3559,7 +3571,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(sysdate() - interval 31 day)",
+                                    Some(
+                                        "(sysdate() - interval 31 day)",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3588,7 +3602,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(conv(10,10,2))",
+                                    Some(
+                                        "(conv(10,10,2))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3615,7 +3631,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(trim(_utf8mb4\\'{} \\'))",
+                                    Some(
+                                        "(trim(_utf8mb4\\'{} \\'))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3640,7 +3658,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(trim(_utf8mb4\\'x-small   \\'))",
+                                    Some(
+                                        "(trim(_utf8mb4\\'x-small   \\'))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },
@@ -3663,7 +3683,9 @@ fn function_expression_defaults_are_described_as_dbgenerated(api: TestApi) {
                         default: Some(
                             DefaultValue {
                                 kind: DbGenerated(
-                                    "(trim(_utf8mb4\\' \\'))",
+                                    Some(
+                                        "(trim(_utf8mb4\\' \\'))",
+                                    ),
                                 ),
                                 constraint_name: None,
                             },

diff --git a/migration-engine/connectors/sql-migration-connector/src/sql_renderer/mssql_renderer.rs b/migration-engine/connectors/sql-migration-connector/src/sql_renderer/mssql_renderer.rs
@@ -37,6 +37,7 @@ impl MssqlFlavour {
         } else {
             column
                 .default()
+                .filter(|d| !matches!(d.kind(), sql::DefaultKind::DbGenerated(None)))
                 .map(|default| {
                     // named constraints
                     let constraint_name = default
@@ -513,7 +514,7 @@ fn escape_string_literal(s: &str) -> String {
 
 fn render_default(default: &sql::DefaultValue) -> Cow<'_, str> {
     match default.kind() {
-        sql::DefaultKind::DbGenerated(val) => val.as_str().into(),
+        sql::DefaultKind::DbGenerated(val) => val.as_ref().unwrap().as_str().into(),
         sql::DefaultKind::Value(PrismaValue::String(val)) | sql::DefaultKind::Value(PrismaValue::Enum(val)) => {
             Quoted::mssql_string(escape_string_literal(val)).to_string().into()
         }

diff --git a/migration-engine/connectors/sql-migration-connector/src/sql_renderer/mysql_renderer.rs b/migration-engine/connectors/sql-migration-connector/src/sql_renderer/mysql_renderer.rs
@@ -27,7 +27,7 @@ impl MysqlFlavour {
             default: col
                 .default()
                 .filter(|default| {
-                    !matches!(default.kind(),  DefaultKind::Sequence(_))
+                    !matches!(default.kind(),  DefaultKind::Sequence(_) | DefaultKind::DbGenerated(None))
                     // We do not want to render JSON defaults because
                     // they are not supported by MySQL.
                     && !matches!(col.column_type_family(), ColumnTypeFamily::Json)
@@ -488,8 +488,8 @@ impl MysqlAlterColumn {
         // @default(dbgenerated()) does not give us the information in the prisma schema, so we have to
         // transfer it from the introspected current state of the database.
         let new_default = match defaults {
-            (Some(DefaultKind::DbGenerated(previous)), Some(DefaultKind::DbGenerated(next)))
-                if next.is_empty() && !previous.is_empty() =>
+            (Some(DefaultKind::DbGenerated(Some(previous))), Some(DefaultKind::DbGenerated(next)))
+                if (next.is_none() || next.as_deref() == Some("")) && !previous.is_empty() =>
             {
                 Some(DefaultValue::db_generated(previous.clone()))
             }
@@ -502,7 +502,7 @@ impl MysqlAlterColumn {
 
 fn render_default<'a>(column: ColumnWalker<'a>, default: &'a DefaultValue) -> Cow<'a, str> {
     match default.kind() {
-        DefaultKind::DbGenerated(val) => val.as_str().into(),
+        DefaultKind::DbGenerated(Some(val)) => val.as_str().into(),
         DefaultKind::Value(PrismaValue::String(val)) | DefaultKind::Value(PrismaValue::Enum(val)) => {
             Quoted::mysql_string(escape_string_literal(val)).to_string().into()
         }
@@ -519,6 +519,6 @@ fn render_default<'a>(column: ColumnWalker<'a>, default: &'a DefaultValue) -> Co
             Quoted::mysql_string(dt.to_rfc3339()).to_string().into()
         }
         DefaultKind::Value(val) => val.to_string().into(),
-        DefaultKind::Sequence(_) | DefaultKind::UniqueRowid => unreachable!(),
+        DefaultKind::DbGenerated(None) | DefaultKind::Sequence(_) | DefaultKind::UniqueRowid => unreachable!(),
     }
 }
diff --git a/migration-engine/connectors/sql-migration-connector/src/sql_renderer/postgres_renderer.rs b/migration-engine/connectors/sql-migration-connector/src/sql_renderer/postgres_renderer.rs
@@ -796,14 +796,14 @@ fn render_default<'a>(default: &'a DefaultValue, full_data_type: &str) -> Cow<'a
     }
 
     match default.kind() {
-        DefaultKind::DbGenerated(val) => Cow::Borrowed(val.as_str()),
+        DefaultKind::DbGenerated(Some(val)) => Cow::Borrowed(val.as_str()),
         DefaultKind::Value(PrismaValue::String(val)) | DefaultKind::Value(PrismaValue::Enum(val)) => {
             format!("'{}'", escape_string_literal(val)).into()
         }
         DefaultKind::Now => "CURRENT_TIMESTAMP".into(),
         DefaultKind::Value(value) => render_constant_default(value, full_data_type),
         DefaultKind::UniqueRowid => "unique_rowid()".into(),
-        DefaultKind::Sequence(_) => Default::default(),
+        DefaultKind::Sequence(_) | DefaultKind::DbGenerated(None) => Default::default(),
     }
 }
 

diff --git a/migration-engine/connectors/sql-migration-connector/src/sql_renderer/sqlite_renderer.rs b/migration-engine/connectors/sql-migration-connector/src/sql_renderer/sqlite_renderer.rs
@@ -311,7 +311,12 @@ fn render_column<'a>(column: &ColumnWalker<'a>) -> ddl::Column<'a> {
         autoincrement: column.is_single_primary_key() && column.column_type_family().is_int(),
         default: column
             .default()
-            .filter(|default| !matches!(default.kind(), DefaultKind::Sequence(_)))
+            .filter(|default| {
+                !matches!(
+                    default.kind(),
+                    DefaultKind::Sequence(_) | DefaultKind::DbGenerated(None)
+                )
+            })
             .map(render_default),
         name: column.name().into(),
         not_null: !column.arity().is_nullable(),
@@ -322,7 +327,7 @@ fn render_column<'a>(column: &ColumnWalker<'a>) -> ddl::Column<'a> {
 
 fn render_default(default: &DefaultValue) -> Cow<'_, str> {
     match default.kind() {
-        DefaultKind::DbGenerated(val) => val.as_str().into(),
+        DefaultKind::DbGenerated(Some(val)) => val.as_str().into(),
         DefaultKind::Value(PrismaValue::String(val)) | DefaultKind::Value(PrismaValue::Enum(val)) => {
             Quoted::sqlite_string(escape_quotes(val)).to_string().into()
         }
@@ -334,6 +339,6 @@ fn render_default(default: &DefaultValue) -> Cow<'_, str> {
         DefaultKind::Now => "CURRENT_TIMESTAMP".into(),
         DefaultKind::Value(PrismaValue::DateTime(val)) => Quoted::sqlite_string(val).to_string().into(),
         DefaultKind::Value(val) => val.to_string().into(),
-        DefaultKind::Sequence(_) | DefaultKind::UniqueRowid => unreachable!(),
+        DefaultKind::DbGenerated(None) | DefaultKind::Sequence(_) | DefaultKind::UniqueRowid => unreachable!(),
     }
 }
diff --git a/migration-engine/connectors/sql-migration-connector/src/sql_schema_calculator.rs b/migration-engine/connectors/sql-migration-connector/src/sql_schema_calculator.rs
@@ -462,9 +462,8 @@ fn push_column_for_builtin_scalar_type(
     let default: Option<ColumnDefault> = field.default_value().map(|v| {
         let column_default = {
             if v.is_dbgenerated() {
-                unwrap_dbgenerated(v.value())
-                    .map(|v| ColumnDefault::Available(sql::DefaultValue::new(sql::DefaultKind::DbGenerated(v))))
-                    .unwrap_or(ColumnDefault::NA)
+                let value = unwrap_dbgenerated(v.value());
+                ColumnDefault::Available(sql::DefaultValue::new(sql::DefaultKind::DbGenerated(value)))
             } else if v.is_now() {
                 ColumnDefault::Available(sql::DefaultValue::now())
             } else if v.is_autoincrement() {

diff --git a/migration-engine/connectors/sql-migration-connector/src/sql_schema_differ/column.rs b/migration-engine/connectors/sql-migration-connector/src/sql_schema_differ/column.rs
@@ -92,6 +92,7 @@ fn defaults_match(cols: Pair<ColumnWalker<'_>>, flavour: &dyn SqlFlavour) -> boo
         (Some(DefaultKind::DbGenerated(_)), Some(DefaultKind::Value(_))) => false,
         (Some(DefaultKind::DbGenerated(_)), Some(DefaultKind::Now)) => false,
         (Some(DefaultKind::DbGenerated(_)), None) => false,
+        (_, Some(DefaultKind::DbGenerated(None))) => true,
 
         (Some(DefaultKind::Sequence(_)), None) => true, // sequences are dropped separately
         (Some(DefaultKind::Sequence(_)), Some(DefaultKind::Value(_))) => false,
@@ -104,7 +105,7 @@ fn defaults_match(cols: Pair<ColumnWalker<'_>>, flavour: &dyn SqlFlavour) -> boo
         (None, Some(DefaultKind::Value(_))) => false,
         (None, Some(DefaultKind::Now)) => false,
 
-        (Some(DefaultKind::DbGenerated(prev)), Some(DefaultKind::DbGenerated(next))) => {
+        (Some(DefaultKind::DbGenerated(Some(prev))), Some(DefaultKind::DbGenerated(Some(next)))) => {
             (prev.eq_ignore_ascii_case(next)) && names_match
         }
         (_, Some(DefaultKind::DbGenerated(_))) => false,

diff --git a/...nectors/sql-migration-connector/src/sql_schema_differ/sql_schema_differ_flavour/sqlite.rs b/...nectors/sql-migration-connector/src/sql_schema_differ/sql_schema_differ_flavour/sqlite.rs
@@ -18,6 +18,10 @@ impl SqlSchemaDifferFlavour for SqliteFlavour {
         false
     }
 
+    fn column_autoincrement_changed(&self, _columns: Pair<ColumnWalker<'_>>) -> bool {
+        false
+    }
+
     fn column_type_change(&self, differ: Pair<ColumnWalker<'_>>) -> Option<ColumnTypeChange> {
         match (differ.previous.column_type_family(), differ.next.column_type_family()) {
             (a, b) if a == b => None,

diff --git a/migration-engine/migration-engine-tests/src/assertions.rs b/migration-engine/migration-engine-tests/src/assertions.rs
@@ -423,7 +423,7 @@ impl<'a> ColumnAssertion<'a> {
         let found = self.column.default();
 
         match found.map(|d| d.kind()) {
-            Some(DefaultKind::DbGenerated(val)) => assert!(
+            Some(DefaultKind::DbGenerated(Some(val))) => assert!(
                 val == expected,
                 "Assertion failed. Expected the default value for `{}` to be dbgenerated with `{:?}`, got `{:?}`",
                 self.column.name(),

diff --git a/migration-engine/migration-engine-tests/tests/migrations/defaults.rs b/migration-engine/migration-engine-tests/tests/migrations/defaults.rs
@@ -293,6 +293,10 @@ fn schemas_with_dbgenerated_work(api: TestApi) {
     "#;
 
     api.schema_push_w_datasource(dm1).send().assert_green();
+    api.schema_push_w_datasource(dm1)
+        .send()
+        .assert_green()
+        .assert_no_steps();
 }
 
 #[test_connector(tags(Mysql8, Mariadb), exclude(Vitess))]

diff --git a/migration-engine/migration-engine-tests/tests/migrations/postgres.rs b/migration-engine/migration-engine-tests/tests/migrations/postgres.rs
@@ -724,3 +724,32 @@ fn bigint_defaults_work(api: TestApi) {
     api.schema_push(schema).send().assert_green();
     api.schema_push(schema).send().assert_green().assert_no_steps();
 }
+
+// https://github.com/prisma/prisma/issues/14799
+#[test_connector(tags(Postgres12), exclude(CockroachDb))]
+fn dbgenerated_on_generated_columns_is_idempotent(api: TestApi) {
+    let sql = r#"
+        CREATE TABLE "table" (
+         "id" TEXT NOT NULL,
+         "hereBeDragons" TEXT NOT NULL GENERATED ALWAYS AS ('this row ID is: '::text || "id") STORED,
+
+         CONSTRAINT "table_pkey" PRIMARY KEY ("id")
+        );
+    "#;
+
+    api.raw_cmd(sql);
+
+    let schema = r#"
+        datasource db {
+            provider = "postgresql"
+            url = env("TEST_DATABASE_URL")
+        }
+
+        model table {
+            id String @id
+            hereBeDragons String @default(dbgenerated())
+        }
+    "#;
+
+    api.schema_push(schema).send().assert_green().assert_no_steps();
+}