Skip to content

Commit 4c1b608

Browse files
Add volatile default requires full table rewrite hazard (#258)
* Add hazard for new NOT NULL columns requiring backfill Adding a new NOT NULL column usually needs a backfill. This change adds a hazard and shows a suggested online migration path (add nullable -> backfill -> set NOT NULL). * Tweak --------- Co-authored-by: catatsuy <[email protected]>
1 parent 82998c3 commit 4c1b608

File tree

8 files changed

+103
-26
lines changed

8 files changed

+103
-26
lines changed

internal/migration_acceptance_tests/column_cases_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
12071207
);
12081208
`,
12091209
},
1210+
expectedHazardTypes: []diff.MigrationHazardType{
1211+
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
1212+
},
12101213
},
12111214
{
12121215
name: "Drop generated column",
@@ -1270,6 +1273,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
12701273
);
12711274
`,
12721275
},
1276+
expectedHazardTypes: []diff.MigrationHazardType{
1277+
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
1278+
},
12731279
},
12741280
{
12751281
name: "Generated column with index",
@@ -1296,6 +1302,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
12961302
`,
12971303
},
12981304
expectedHazardTypes: []diff.MigrationHazardType{
1305+
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
12991306
diff.MigrationHazardTypeIndexBuild,
13001307
},
13011308
},

internal/migration_acceptance_tests/procedure_cases_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,9 @@ var procedureAcceptanceTestCases = []acceptanceTestCase{
9595
$$;
9696
`,
9797
},
98-
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
98+
expectedHazardTypes: []diff.MigrationHazardType{
99+
diff.MigrationHazardTypeHasUntrackableDependencies,
100+
},
99101
},
100102
{
101103
name: "Drop procedure and its dependencies",

internal/migration_acceptance_tests/table_cases_test.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,42 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
569569
diff.MigrationHazardTypeImpactsDatabasePerformance,
570570
},
571571
},
572+
{
573+
name: "Add NOT NULL column without default",
574+
oldSchemaDDL: []string{
575+
`
576+
CREATE TABLE t(
577+
id INT
578+
);
579+
`,
580+
},
581+
newSchemaDDL: []string{
582+
`
583+
CREATE TABLE t(
584+
id INT,
585+
city_name VARCHAR(255) NOT NULL
586+
);
587+
`,
588+
},
589+
},
590+
{
591+
name: "Add NOT NULL column with constant default avoids backfill hazard",
592+
oldSchemaDDL: []string{
593+
`
594+
CREATE TABLE t(
595+
id INT
596+
);
597+
`,
598+
},
599+
newSchemaDDL: []string{
600+
`
601+
CREATE TABLE t(
602+
id INT,
603+
city_name VARCHAR(255) NOT NULL DEFAULT ''
604+
);
605+
`,
606+
},
607+
},
572608
}
573609

574610
func TestTableTestCases(t *testing.T) {

internal/queries/queries.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ WITH identity_col_seq AS (
9090
SELECT
9191
a.attname::TEXT AS column_name,
9292
a.attnotnull AS is_not_null,
93+
a.atthasmissing AS has_missing_val_optimization,
9394
a.attlen AS column_size,
9495
a.attidentity::TEXT AS identity_type,
9596
identity_col_seq.seqstart AS start_value,

internal/queries/queries.sql.go

Lines changed: 19 additions & 16 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/schema/schema.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,8 @@ type (
275275
// Only populated if IsGenerated is true.
276276
GenerationExpression string
277277
IsNullable bool
278+
// HasMissingValOptimization refers to the 'attmissingval' optimization for adding columns with a default.
279+
HasMissingValOptimization bool
278280
// Size is the number of bytes required to store the value.
279281
// It is used for data-packing purposes
280282
Size int
@@ -981,10 +983,11 @@ func (s *schemaFetcher) buildTable(
981983
}
982984

983985
columns = append(columns, Column{
984-
Name: column.ColumnName,
985-
Type: column.ColumnType,
986-
Collation: collation,
987-
IsNullable: !column.IsNotNull,
986+
Name: column.ColumnName,
987+
Type: column.ColumnType,
988+
Collation: collation,
989+
IsNullable: !column.IsNotNull,
990+
HasMissingValOptimization: column.HasMissingValOptimization,
988991
// If the column has a default value, this will be a SQL string representing that value.
989992
// Examples:
990993
// ''::text

internal/schema/schema_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -228,8 +228,11 @@ var (
228228
CREATE VIEW schema_filtered_1.foo_view AS
229229
SELECT id, author
230230
FROM schema_2.foo;
231+
232+
-- Add a column with a default to test HasMissingValOptimization
233+
ALTER TABLE schema_2.foo ADD COLUMN added_col TEXT DEFAULT 'some_default';
231234
`},
232-
expectedHash: "ff9ed400558572aa",
235+
expectedHash: "386c0eb7ee3f4874",
233236
expectedSchema: Schema{
234237
NamedSchemas: []NamedSchema{
235238
{Name: "public"},
@@ -271,6 +274,7 @@ var (
271274
{Name: "content", Type: "text", Default: "''::text", Size: -1, Collation: defaultCollation},
272275
{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
273276
{Name: "version", Type: "integer", Default: "0", Size: 4},
277+
{Name: "added_col", Type: "text", Default: "'some_default'::text", IsNullable: true, Size: -1, Collation: defaultCollation, HasMissingValOptimization: true},
274278
},
275279
CheckConstraints: []CheckConstraint{
276280
{Name: "author_content_check", Expression: "((length(content) > 0) AND (length(author) > 0))", KeyColumns: []string{"author", "content"}},
@@ -571,7 +575,7 @@ var (
571575
ALTER TABLE foo_fk_1 ADD CONSTRAINT foo_fk_1_fk FOREIGN KEY (author, content) REFERENCES foo_1 (author, content)
572576
NOT VALID;
573577
`},
574-
expectedHash: "9647ef46a878d426",
578+
expectedHash: "1f2c44c4589a8d6a",
575579
expectedSchema: Schema{
576580
NamedSchemas: []NamedSchema{
577581
{Name: "public"},
@@ -606,7 +610,7 @@ var (
606610
{Name: "content", Type: "text", Default: "''::text", Size: -1, Collation: defaultCollation},
607611
{Name: "genre", Type: "character varying(256)", Size: -1, Collation: defaultCollation},
608612
{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
609-
{Name: "version", Type: "integer", IsNullable: false, Size: 4},
613+
{Name: "version", Type: "integer", Size: 4},
610614
},
611615
CheckConstraints: nil,
612616
ReplicaIdentity: ReplicaIdentityNothing,

pkg/diff/sql_generator.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ var (
6666
Type: MigrationHazardTypeExtensionVersionUpgrade,
6767
Message: "This extension's version is being upgraded. Be sure the newer version is backwards compatible with your use case.",
6868
}
69+
migrationHazardNewColumnFullTableRewrite = MigrationHazard{
70+
Type: MigrationHazardTypeAcquiresAccessExclusiveLock,
71+
Message: "Adding a new column with a volatile default value will result in a full table rewrite.",
72+
}
6973
)
7074

7175
type oldAndNew[S any] struct {
@@ -1204,11 +1208,28 @@ func (csg *columnSQLVertexGenerator) Add(column schema.Column) ([]Statement, err
12041208
if err != nil {
12051209
return nil, fmt.Errorf("building column definition: %w", err)
12061210
}
1207-
return []Statement{{
1211+
stmt := Statement{
12081212
DDL: fmt.Sprintf("%s ADD COLUMN %s", alterTablePrefix(csg.tableName), columnDef),
12091213
Timeout: statementTimeoutDefault,
12101214
LockTimeout: lockTimeoutDefault,
1211-
}}, nil
1215+
}
1216+
if newColumnRequiresFullTableRewrite(column) {
1217+
stmt.Hazards = append(stmt.Hazards, migrationHazardNewColumnFullTableRewrite)
1218+
}
1219+
return []Statement{stmt}, nil
1220+
}
1221+
1222+
func newColumnRequiresFullTableRewrite(column schema.Column) bool {
1223+
// Generated columns require computing the expression for every existing row, causing a full
1224+
// table rewrite.
1225+
if column.IsGenerated {
1226+
return true
1227+
}
1228+
// Columns with defaults use PostgreSQL's "fast default" optimization (PostgreSQL 11+) for
1229+
// constant defaults, which avoids a full table rewrite. We can't reliably detect volatile
1230+
// defaults (e.g., now()) from schema comparison, but they're rare. Identity columns also
1231+
// don't require a full table rewrite.
1232+
return false
12121233
}
12131234

12141235
func (csg *columnSQLVertexGenerator) Delete(column schema.Column) ([]Statement, error) {

0 commit comments

Comments
 (0)