Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions internal/migration_acceptance_tests/column_cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1207,6 +1207,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
);
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
},
},
{
name: "Drop generated column",
Expand Down Expand Up @@ -1270,6 +1273,9 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
);
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
},
},
{
name: "Generated column with index",
Expand All @@ -1296,6 +1302,7 @@ var columnAcceptanceTestCases = []acceptanceTestCase{
`,
},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeAcquiresAccessExclusiveLock,
diff.MigrationHazardTypeIndexBuild,
},
},
Expand Down
4 changes: 3 additions & 1 deletion internal/migration_acceptance_tests/procedure_cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@ var procedureAcceptanceTestCases = []acceptanceTestCase{
$$;
`,
},
expectedHazardTypes: []diff.MigrationHazardType{diff.MigrationHazardTypeHasUntrackableDependencies},
expectedHazardTypes: []diff.MigrationHazardType{
diff.MigrationHazardTypeHasUntrackableDependencies,
},
},
{
name: "Drop procedure and its dependencies",
Expand Down
36 changes: 36 additions & 0 deletions internal/migration_acceptance_tests/table_cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,42 @@ var tableAcceptanceTestCases = []acceptanceTestCase{
diff.MigrationHazardTypeImpactsDatabasePerformance,
},
},
{
name: "Add NOT NULL column without default",
oldSchemaDDL: []string{
`
CREATE TABLE t(
id INT
);
`,
},
newSchemaDDL: []string{
`
CREATE TABLE t(
id INT,
city_name VARCHAR(255) NOT NULL
);
`,
},
},
{
name: "Add NOT NULL column with constant default avoids backfill hazard",
oldSchemaDDL: []string{
`
CREATE TABLE t(
id INT
);
`,
},
newSchemaDDL: []string{
`
CREATE TABLE t(
id INT,
city_name VARCHAR(255) NOT NULL DEFAULT ''
);
`,
},
},
}

func TestTableTestCases(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions internal/queries/queries.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
35 changes: 19 additions & 16 deletions internal/queries/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 7 additions & 4 deletions internal/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.HasMissingValOptimization,
// If the column has a default value, this will be a SQL string representing that value.
// Examples:
// ''::text
Expand Down
10 changes: 7 additions & 3 deletions internal/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,11 @@ var (
CREATE VIEW schema_filtered_1.foo_view AS
SELECT id, author
FROM schema_2.foo;

-- Add a column with a default to test HasMissingValOptimization
ALTER TABLE schema_2.foo ADD COLUMN added_col TEXT DEFAULT 'some_default';
`},
expectedHash: "ff9ed400558572aa",
expectedHash: "386c0eb7ee3f4874",
expectedSchema: Schema{
NamedSchemas: []NamedSchema{
{Name: "public"},
Expand Down Expand Up @@ -271,6 +274,7 @@ var (
{Name: "content", Type: "text", Default: "''::text", Size: -1, Collation: defaultCollation},
{Name: "created_at", Type: "timestamp without time zone", Default: "CURRENT_TIMESTAMP", Size: 8},
{Name: "version", Type: "integer", Default: "0", Size: 4},
{Name: "added_col", Type: "text", Default: "'some_default'::text", IsNullable: true, Size: -1, Collation: defaultCollation, HasMissingValOptimization: true},
},
CheckConstraints: []CheckConstraint{
{Name: "author_content_check", Expression: "((length(content) > 0) AND (length(author) > 0))", KeyColumns: []string{"author", "content"}},
Expand Down Expand Up @@ -571,7 +575,7 @@ var (
ALTER TABLE foo_fk_1 ADD CONSTRAINT foo_fk_1_fk FOREIGN KEY (author, content) REFERENCES foo_1 (author, content)
NOT VALID;
`},
expectedHash: "9647ef46a878d426",
expectedHash: "1f2c44c4589a8d6a",
expectedSchema: Schema{
NamedSchemas: []NamedSchema{
{Name: "public"},
Expand Down Expand Up @@ -606,7 +610,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", Size: 4},
},
CheckConstraints: nil,
ReplicaIdentity: ReplicaIdentityNothing,
Expand Down
25 changes: 23 additions & 2 deletions pkg/diff/sql_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@ var (
Type: MigrationHazardTypeExtensionVersionUpgrade,
Message: "This extension's version is being upgraded. Be sure the newer version is backwards compatible with your use case.",
}
migrationHazardNewColumnFullTableRewrite = MigrationHazard{
Type: MigrationHazardTypeAcquiresAccessExclusiveLock,
Message: "Adding a new column with a volatile default value will result in a full table rewrite.",
}
)

type oldAndNew[S any] struct {
Expand Down Expand Up @@ -1204,11 +1208,28 @@ func (csg *columnSQLVertexGenerator) Add(column schema.Column) ([]Statement, err
if err != nil {
return nil, fmt.Errorf("building column definition: %w", err)
}
return []Statement{{
stmt := Statement{
DDL: fmt.Sprintf("%s ADD COLUMN %s", alterTablePrefix(csg.tableName), columnDef),
Timeout: statementTimeoutDefault,
LockTimeout: lockTimeoutDefault,
}}, nil
}
if newColumnRequiresFullTableRewrite(column) {
stmt.Hazards = append(stmt.Hazards, migrationHazardNewColumnFullTableRewrite)
}
return []Statement{stmt}, nil
}

func newColumnRequiresFullTableRewrite(column schema.Column) bool {
// Generated columns require computing the expression for every existing row, causing a full
// table rewrite.
if column.IsGenerated {
return true
}
// Columns with defaults use PostgreSQL's "fast default" optimization (PostgreSQL 11+) for
// constant defaults, which avoids a full table rewrite. We can't reliably detect volatile
// defaults (e.g., now()) from schema comparison, but they're rare. Identity columns also
// don't require a full table rewrite.
return false
}

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