Update logic

bplunkett-stripe · bplunkett-stripe · commit 45119c9e0fc0 · 2025-12-30T16:22:06.000-05:00
diff --git a/internal/migration_acceptance_tests/column_cases_test.go b/internal/migration_acceptance_tests/column_cases_test.go
@@ -86,7 +86,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
 			`,
 		},
 		expectedHazardTypes: []diff.MigrationHazardType{
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 		},
 	},
 	{
diff --git a/internal/migration_acceptance_tests/database_schema_source_cases_test.go b/internal/migration_acceptance_tests/database_schema_source_cases_test.go
@@ -155,8 +155,8 @@ var databaseSchemaSourceTestCases = []acceptanceTestCase{
 		},
 
 		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 			diff.MigrationHazardTypeIndexBuild,
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
 		},
 		expectedDBSchemaDDL: []string{
 			`
diff --git a/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go b/internal/migration_acceptance_tests/foreign_key_constraint_cases_test.go
@@ -866,7 +866,6 @@ var foreignKeyConstraintCases = []acceptanceTestCase{
 
 			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 			diff.MigrationHazardTypeAcquiresShareRowExclusiveLock,
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
 			diff.MigrationHazardTypeIndexBuild,
 			diff.MigrationHazardTypeIndexDropped,
 		},
diff --git a/internal/migration_acceptance_tests/procedure_cases_test.go b/internal/migration_acceptance_tests/procedure_cases_test.go
@@ -96,8 +96,8 @@ var procedureAcceptanceTestCases = []acceptanceTestCase{
 			`,
 		},
 		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 			diff.MigrationHazardTypeHasUntrackableDependencies,
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
 		},
 	},
 	{
diff --git a/internal/migration_acceptance_tests/table_cases_test.go b/internal/migration_acceptance_tests/table_cases_test.go
@@ -541,7 +541,6 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
 			diff.MigrationHazardTypeAuthzUpdate,
 			diff.MigrationHazardTypeDeletesData,
 			diff.MigrationHazardTypeIndexDropped,
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
 			diff.MigrationHazardTypeIndexBuild,
 		},
 	},
@@ -571,7 +570,7 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
 		},
 	},
 	{
-		name: "Add NOT NULL column without default emits backfill hazard",
+		name: "Add NOT NULL column without default",
 		oldSchemaDDL: []string{
 			`
             CREATE TABLE t(
@@ -588,12 +587,7 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
 			`,
 		},
 		expectedHazardTypes: []diff.MigrationHazardType{
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
-		},
-		expectedHazardMessages: []string{
-			"backfill",
-			"Add the column as NULLABLE",
-			"SET NOT NULL",
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 		},
 	},
 	{
diff --git a/internal/queries/queries.sql b/internal/queries/queries.sql
@@ -90,6 +90,7 @@ WITH identity_col_seq AS (
 SELECT
     a.attname::TEXT AS column_name,
     a.attnotnull AS is_not_null,
+    a.atthasmissing as has_missing,
     a.attlen AS column_size,
     a.attidentity::TEXT AS identity_type,
     identity_col_seq.seqstart AS start_value,
diff --git a/internal/queries/queries.sql.go b/internal/queries/queries.sql.go
diff --git a/internal/schema/schema.go b/internal/schema/schema.go
@@ -274,6 +274,8 @@ type (
 		// Only populated if IsGenerated is true.
 		GenerationExpression string
 		IsNullable           bool
+		// HasMissingValOptimization refers to the 'attmissingval' optimization for adding columns with a default.
+		HasMissingValOptimization bool
 		// Size is the number of bytes required to store the value.
 		// It is used for data-packing purposes
 		Size     int
@@ -980,10 +982,11 @@ func (s *schemaFetcher) buildTable(
 		}
 
 		columns = append(columns, Column{
-			Name:       column.ColumnName,
-			Type:       column.ColumnType,
-			Collation:  collation,
-			IsNullable: !column.IsNotNull,
+			Name:                      column.ColumnName,
+			Type:                      column.ColumnType,
+			Collation:                 collation,
+			IsNullable:                !column.IsNotNull,
+			HasMissingValOptimization: column.HasMissing,
 			// If the column has a default value, this will be a SQL string representing that value.
 			// Examples:
 			//   ''::text
diff --git a/internal/schema/schema_test.go b/internal/schema/schema_test.go
@@ -606,7 +606,7 @@ var (
 							{Name: "content", Type: "text", Default: "''::text", Size: -1, Collation: defaultCollation},
 							{Name: "genre", Type: "character varying(256)", Size: -1, Collation: defaultCollation},
 							{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
-							{Name: "version", Type: "integer", IsNullable: false, Size: 4},
+							{Name: "version", Type: "integer", IsNullable: false, , Size: 4},
 						},
 						CheckConstraints: nil,
 						ReplicaIdentity:  ReplicaIdentityNothing,
diff --git a/pkg/diff/plan.go b/pkg/diff/plan.go
@@ -10,19 +10,18 @@ import (
 type MigrationHazardType = string
 
 const (
-	MigrationHazardTypeAcquiresAccessExclusiveLock      MigrationHazardType = "ACQUIRES_ACCESS_EXCLUSIVE_LOCK"
-	MigrationHazardTypeAcquiresShareLock                MigrationHazardType = "ACQUIRES_SHARE_LOCK"
-	MigrationHazardTypeAcquiresShareRowExclusiveLock    MigrationHazardType = "ACQUIRES_SHARE_ROW_EXCLUSIVE_LOCK"
-	MigrationHazardTypeCorrectness                      MigrationHazardType = "CORRECTNESS"
-	MigrationHazardTypeDeletesData                      MigrationHazardType = "DELETES_DATA"
-	MigrationHazardTypeHasUntrackableDependencies       MigrationHazardType = "HAS_UNTRACKABLE_DEPENDENCIES"
-	MigrationHazardTypeIndexBuild                       MigrationHazardType = "INDEX_BUILD"
-	MigrationHazardTypeIndexDropped                     MigrationHazardType = "INDEX_DROPPED"
-	MigrationHazardTypeImpactsDatabasePerformance       MigrationHazardType = "IMPACTS_DATABASE_PERFORMANCE"
-	MigrationHazardTypeIsUserGenerated                  MigrationHazardType = "IS_USER_GENERATED"
-	MigrationHazardTypeExtensionVersionUpgrade          MigrationHazardType = "UPGRADING_EXTENSION_VERSION"
-	MigrationHazardTypeAuthzUpdate                      MigrationHazardType = "AUTHZ_UPDATE"
-	MigrationHazardTypeNewNotNullColumnRequiresBackfill MigrationHazardType = "NEW_NOT_NULL_COLUMN_REQUIRES_BACKFILL"
+	MigrationHazardTypeAcquiresAccessExclusiveLock   MigrationHazardType = "ACQUIRES_ACCESS_EXCLUSIVE_LOCK"
+	MigrationHazardTypeAcquiresShareLock             MigrationHazardType = "ACQUIRES_SHARE_LOCK"
+	MigrationHazardTypeAcquiresShareRowExclusiveLock MigrationHazardType = "ACQUIRES_SHARE_ROW_EXCLUSIVE_LOCK"
+	MigrationHazardTypeCorrectness                   MigrationHazardType = "CORRECTNESS"
+	MigrationHazardTypeDeletesData                   MigrationHazardType = "DELETES_DATA"
+	MigrationHazardTypeHasUntrackableDependencies    MigrationHazardType = "HAS_UNTRACKABLE_DEPENDENCIES"
+	MigrationHazardTypeIndexBuild                    MigrationHazardType = "INDEX_BUILD"
+	MigrationHazardTypeIndexDropped                  MigrationHazardType = "INDEX_DROPPED"
+	MigrationHazardTypeImpactsDatabasePerformance    MigrationHazardType = "IMPACTS_DATABASE_PERFORMANCE"
+	MigrationHazardTypeIsUserGenerated               MigrationHazardType = "IS_USER_GENERATED"
+	MigrationHazardTypeExtensionVersionUpgrade       MigrationHazardType = "UPGRADING_EXTENSION_VERSION"
+	MigrationHazardTypeAuthzUpdate                   MigrationHazardType = "AUTHZ_UPDATE"
 )
 
 // MigrationHazard represents a hazard that a statement poses to a database
diff --git a/pkg/diff/sql_generator.go b/pkg/diff/sql_generator.go
@@ -66,13 +66,9 @@ var (
 		Type:    MigrationHazardTypeExtensionVersionUpgrade,
 		Message: "This extension's version is being upgraded. Be sure the newer version is backwards compatible with your use case.",
 	}
-	migrationHazardNewNotNullColumnRequiresBackfill = MigrationHazard{
-		Type: MigrationHazardTypeNewNotNullColumnRequiresBackfill,
-		Message: "Adding a new NOT NULL column without a constant DEFAULT requires a backfill to populate existing rows. " +
-			"Recommended online sequence:\n" +
-			"  1) Add the column as NULLABLE (no NOT NULL)\n" +
-			"  2) Backfill existing rows in batches via application jobs or scripts\n" +
-			"  3) Re-run pg-schema-diff to enforce NOT NULL using the online CHECK/VALIDATE/SET NOT NULL flow",
+	migrationHazardNewColumnFullTableRewrite = MigrationHazard{
+		Type:    MigrationHazardTypeAcquiresAccessExclusiveLock,
+		Message: "Adding a new column with a volatile default value will result in a full table rewrite.",
 	}
 )
 
@@ -1217,24 +1213,19 @@ func (csg *columnSQLVertexGenerator) Add(column schema.Column) ([]Statement, err
 		Timeout:     statementTimeoutDefault,
 		LockTimeout: lockTimeoutDefault,
 	}
-	if columnRequiresBackfill(column) {
-		stmt.Hazards = append(stmt.Hazards, migrationHazardNewNotNullColumnRequiresBackfill)
+	if newColumnRequiresFullTableRewrite(column) {
+		stmt.Hazards = append(stmt.Hazards, migrationHazardNewColumnFullTableRewrite)
 	}
 	return []Statement{stmt}, nil
 }
 
-func columnRequiresBackfill(column schema.Column) bool {
-	if column.IsNullable {
-		return false
+func newColumnRequiresFullTableRewrite(column schema.Column) bool {
+	if len(column.Default) > 0 || column.IsGenerated || column.Identity != nil {
+		// The column has some sort of computed value. If the missing value optimization is present,
+		// then a full-rewrite is not required.
+		return !column.HasMissingValOptimization
 	}
-	return !columnHasSafeDefault(column)
-}
-
-func columnHasSafeDefault(column schema.Column) bool {
-	if column.Identity != nil || column.IsGenerated {
-		return true
-	}
-	return len(strings.TrimSpace(column.Default)) > 0
+	return false
 }
 
 func (csg *columnSQLVertexGenerator) Delete(column schema.Column) ([]Statement, error) {

Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{`
`86`	`86`	`,
`87`	`87`	`},`
`88`	`88`	`expectedHazardTypes: []diff.MigrationHazardType{`
`89`		`- diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,`
	`89`	`+ diff.MigrationHazardTypeAcquiresAccessExclusiveLock,`
`90`	`90`	`},`
`91`	`91`	`},`
`92`	`92`	`{`
Original file line number	Diff line number	Diff line change
`@@ -155,8 +155,8 @@ var databaseSchemaSourceTestCases = []acceptanceTestCase{`
`155`	`155`	`},`
`156`	`156`
`157`	`157`	`expectedHazardTypes: []diff.MigrationHazardType{`
	`158`	`+ diff.MigrationHazardTypeAcquiresAccessExclusiveLock,`
`158`	`159`	`diff.MigrationHazardTypeIndexBuild,`
`159`		`- diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,`
`160`	`160`	`},`
`161`	`161`	`expectedDBSchemaDDL: []string{`
`162`	`162`	`