Tweak

Author: bplunkett-stripe

README.md

@@ -152,9 +152,6 @@ Apply the schema. Any hazards in the generated plan must be approved
 pg-schema-diff apply --from-dsn "postgres://postgres:postgres@localhost:5432/postgres" --to-dir schema --allow-hazards INDEX_BUILD
 ```
 
-# Stateful migrations
-- Adding a brand new `NOT NULL` column without a constant `DEFAULT` requires backfilling existing rows. pg-schema-diff will emit the `NEW_NOT_NULL_COLUMN_REQUIRES_BACKFILL` hazard and annotate the plan with an online sequence: add the column as nullable, backfill in batches, then rerun pg-schema-diff to flip to `NOT NULL` via the online CHECK/VALIDATE/SET NOT NULL flow.
-
 # Using Library
 Docs to use the library can be found [here](https://pkg.go.dev/github.com/stripe/pg-schema-diff). Check out [the CLI](https://github.com/stripe/pg-schema-diff/tree/main/cmd/pg-schema-diff)
 for an example implementation with the library

cmd/pg-schema-diff/apply_cmd_test.go

@@ -3,7 +3,6 @@ package main
 import (
 	"github.com/stripe/pg-schema-diff/internal/pgdump"
 	"github.com/stripe/pg-schema-diff/internal/pgengine"
-	"github.com/stripe/pg-schema-diff/pkg/diff"
 )
 
 func (suite *cmdTestSuite) TestApplyCmd() {
@@ -88,21 +87,3 @@ func (suite *cmdTestSuite) TestApplyCmd() {
 		})
 	}
 }
-
-func (suite *cmdTestSuite) TestFailIfHazardsNotAllowed() {
-	plan := diff.Plan{
-		Statements: []diff.Statement{{
-			DDL: "ALTER TABLE public.t ADD COLUMN city_name text NOT NULL",
-			Hazards: []diff.MigrationHazard{{
-				Type: diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
-			}},
-		}},
-	}
-
-	err := failIfHazardsNotAllowed(plan, nil)
-	suite.Error(err)
-	suite.Contains(err.Error(), string(diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill))
-
-	err = failIfHazardsNotAllowed(plan, []string{string(diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill)})
-	suite.NoError(err)
-}

internal/migration_acceptance_tests/acceptance_test.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	stdlog "log"
 	"os"
-	"strings"
 	"testing"
 
 	"github.com/google/uuid"
@@ -63,9 +62,6 @@ type (
 		//
 		// If no expectedDBSchemaDDL is specified, the newSchemaDDL will be used
 		expectedDBSchemaDDL []string
-		// expectedHazardMessages is a list of substrings that must be found within the set of hazard messages generated
-		// by the plan.
-		expectedHazardMessages []string
 	}
 )
 
@@ -182,19 +178,6 @@ func runTest(t *testing.T, tc acceptanceTestCase) {
 	}
 	assert.ElementsMatch(t, tc.expectedHazardTypes, getUniqueHazardTypesFromStatements(plan.Statements), prettySprintPlan(plan))
 
-	if len(tc.expectedHazardMessages) > 0 {
-		var hazardMsgs []string
-		for _, stmt := range plan.Statements {
-			for _, hazard := range stmt.Hazards {
-				hazardMsgs = append(hazardMsgs, hazard.Message)
-			}
-		}
-		joinedHazardMsgs := strings.Join(hazardMsgs, "\n")
-		for _, expectedHazardMsg := range tc.expectedHazardMessages {
-			assert.Contains(t, joinedHazardMsgs, expectedHazardMsg, prettySprintPlan(plan))
-		}
-	}
-
 	// Apply the plan
 	require.NoError(t, applyPlan(oldDb, plan), prettySprintPlan(plan))
 

internal/migration_acceptance_tests/column_cases_test.go

@@ -49,6 +49,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Add one column with quoted names",
@@ -67,6 +70,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Add one column with nullability",
@@ -85,9 +91,6 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
-		expectedHazardTypes: []diff.MigrationHazardType{
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
-		},
 	},
 	{
 		name: "Add one column with serial",
@@ -106,6 +109,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Add one column with all options",
@@ -124,6 +130,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Add one column and change ordering",
@@ -149,6 +158,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
                         my_new_column VARCHAR(255) NOT NULL DEFAULT 'a'
                     )
 				`},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Add identity column - always no cycle",
@@ -167,6 +179,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Add identity column - default cycle",
@@ -185,6 +200,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Delete one column",
@@ -1210,6 +1228,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Drop generated column",
@@ -1273,6 +1294,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
             );
 			`,
 		},
+		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
+		},
 	},
 	{
 		name: "Generated column with index",
@@ -1299,6 +1323,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
 			`,
 		},
 		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 			diff.MigrationHazardTypeIndexBuild,
 		},
 	},

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{
 			`

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,
 		},

internal/migration_acceptance_tests/procedure_cases_test.go

@@ -96,8 +96,8 @@ var procedureAcceptanceTestCases = []acceptanceTestCase{
 			`,
 		},
 		expectedHazardTypes: []diff.MigrationHazardType{
+			diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
 			diff.MigrationHazardTypeHasUntrackableDependencies,
-			diff.MigrationHazardTypeNewNotNullColumnRequiresBackfill,
 		},
 	},
 	{

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,
 		},
 	},
 	{

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_val_optimization,
     a.attlen AS column_size,
     a.attidentity::TEXT AS identity_type,
     identity_col_seq.seqstart AS start_value,