diff --git a/internal/diff/diff.go b/internal/diff/diff.go index d1dfb99e..e23b76ee 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -271,6 +271,7 @@ type ddlDiff struct { addedTables []*ir.Table droppedTables []*ir.Table modifiedTables []*tableDiff + allNewTables map[string]*ir.Table addedViews []*ir.View droppedViews []*ir.View modifiedViews []*viewDiff @@ -626,6 +627,8 @@ func GenerateMigrationWithOptions(oldIR, newIR *ir.IR, targetSchema string, qual } } + diff.allNewTables = newTables + // Compare functions across all schemas oldFunctions := make(map[string]*ir.Function) newFunctions := make(map[string]*ir.Function) @@ -1823,7 +1826,7 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto } // Create tables WITHOUT function/domain dependencies first (functions may reference these) - deferredPolicies1, deferredConstraints1 := generateCreateTablesSQL(tablesWithoutDeps, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs) + deferredPolicies1, deferredConstraints1 := generateCreateTablesSQL(tablesWithoutDeps, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs, d.allNewTables) // Build view lookup - needed for detecting functions that depend on views newViewLookup := buildViewLookup(d.addedViews) @@ -1877,7 +1880,7 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateCreateProceduresSQL(d.addedProcedures, targetSchema, collector) // Create tables WITH function/domain dependencies (now that functions and deferred domains exist) - deferredPolicies2, deferredConstraints2 := generateCreateTablesSQL(tablesWithDeps, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs) + deferredPolicies2, deferredConstraints2 := generateCreateTablesSQL(tablesWithDeps, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs, d.allNewTables) // Emit COMMENT ON SEQUENCE for sequences created implicitly via CREATE TABLE (SERIAL/BIGSERIAL). // These were skipped from addedSequences but their comments must still be deployed. diff --git a/internal/diff/qualify_schema_test.go b/internal/diff/qualify_schema_test.go index fa8ffcab..a3d856bc 100644 --- a/internal/diff/qualify_schema_test.go +++ b/internal/diff/qualify_schema_test.go @@ -55,7 +55,7 @@ func TestQualifySchema_TableAndColumnType(t *testing.T) { } empty := map[string]bool{} - def, _ := generateTableSQL(table, "public", false, empty, empty, empty) + def, _ := generateTableSQL(table, "public", false, empty, empty, empty, nil) if !strings.Contains(def, "CREATE TABLE IF NOT EXISTS account (") { t.Errorf("default should use the bare table name: %q", def) } @@ -63,7 +63,7 @@ func TestQualifySchema_TableAndColumnType(t *testing.T) { t.Errorf("default should not qualify the target schema: %q", def) } - qualified, _ := generateTableSQL(table, "public", true, empty, empty, empty) + qualified, _ := generateTableSQL(table, "public", true, empty, empty, empty, nil) if !strings.Contains(qualified, "CREATE TABLE IF NOT EXISTS public.account (") { t.Errorf("forced qualification should qualify the table name: %q", qualified) } diff --git a/internal/diff/table.go b/internal/diff/table.go index acc7f2a0..079a2c75 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -413,6 +413,7 @@ func generateCreateTablesSQL( existingTables map[string]bool, shouldDeferPolicy func(*ir.RLSPolicy) bool, suppressedInlineFKs map[string]bool, + allNewTables map[string]*ir.Table, ) ([]*ir.RLSPolicy, []*deferredConstraint) { var deferredPolicies []*ir.RLSPolicy var deferredConstraints []*deferredConstraint @@ -421,7 +422,7 @@ func generateCreateTablesSQL( // Process tables in the provided order (already topologically sorted) for _, table := range tables { // Create the table, deferring FK constraints that reference not-yet-created tables - sql, tableDeferred := generateTableSQL(table, targetSchema, collector.qualifySchema, createdTables, existingTables, suppressedInlineFKs) + sql, tableDeferred := generateTableSQL(table, targetSchema, collector.qualifySchema, createdTables, existingTables, suppressedInlineFKs, allNewTables) deferredConstraints = append(deferredConstraints, tableDeferred...) // Create context for this statement @@ -759,7 +760,7 @@ func generateDropTablesSQL(tables []*ir.Table, targetSchema string, collector *d } // generateTableSQL generates CREATE TABLE statement and returns any deferred FK constraints -func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, createdTables map[string]bool, existingTables map[string]bool, suppressedInlineFKs map[string]bool) (string, []*deferredConstraint) { +func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, createdTables map[string]bool, existingTables map[string]bool, suppressedInlineFKs map[string]bool, allTables map[string]*ir.Table) (string, []*deferredConstraint) { // Only include table name without schema if it's in the target schema (unless // forced qualification is on). tableName := ir.QualifyEntityNameWithQuotesMode(table.Schema, table.Name, targetSchema, qualifySchema) @@ -774,7 +775,44 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, } parentName := ir.QualifyEntityNameWithQuotesMode(parentSchema, table.PartitionOf, targetSchema, qualifySchema) - // Include child-specific constraints in the PARTITION OF statement. + // Include child-specific column overrides and constraints in the PARTITION OF statement. + var elementParts []string + + // Detect per-child column overrides (DEFAULT, NOT NULL) by comparing against the parent. + parentKey := parentSchema + "." + table.PartitionOf + if parentTable, ok := allTables[parentKey]; ok { + parentCols := make(map[string]*ir.Column, len(parentTable.Columns)) + for _, col := range parentTable.Columns { + parentCols[col.Name] = col + } + for _, col := range table.Columns { + parentCol := parentCols[col.Name] + if parentCol == nil { + continue + } + var overrides []string + childDefault := "" + parentDefault := "" + if col.DefaultValue != nil { + childDefault = *col.DefaultValue + } + if parentCol.DefaultValue != nil { + parentDefault = *parentCol.DefaultValue + } + if childDefault != parentDefault { + if col.DefaultValue != nil { + overrides = append(overrides, fmt.Sprintf("DEFAULT %s", *col.DefaultValue)) + } + } + if !col.IsNullable && parentCol.IsNullable { + overrides = append(overrides, "NOT NULL") + } + if len(overrides) > 0 { + elementParts = append(elementParts, fmt.Sprintf(" %s %s", ir.QuoteIdentifier(col.Name), strings.Join(overrides, " "))) + } + } + } + inlineConstraints := getInlineConstraintsForTable(table) var constraintParts []string var deferred []*deferredConstraint @@ -800,10 +838,11 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, createPrefix = "CREATE UNLOGGED TABLE IF NOT EXISTS" } + allParts := append(elementParts, constraintParts...) var sql string - if len(constraintParts) > 0 { + if len(allParts) > 0 { sql = fmt.Sprintf("%s %s PARTITION OF %s (\n%s\n) %s;", - createPrefix, tableName, parentName, strings.Join(constraintParts, ",\n"), table.PartitionBound) + createPrefix, tableName, parentName, strings.Join(allParts, ",\n"), table.PartitionBound) } else { sql = fmt.Sprintf("%s %s PARTITION OF %s %s;", createPrefix, tableName, parentName, table.PartitionBound) } diff --git a/testdata/diff/create_table/issue_499_partition_column_overrides/diff.sql b/testdata/diff/create_table/issue_499_partition_column_overrides/diff.sql new file mode 100644 index 00000000..55f62f06 --- /dev/null +++ b/testdata/diff/create_table/issue_499_partition_column_overrides/diff.sql @@ -0,0 +1,4 @@ +CREATE TABLE IF NOT EXISTS orders_us PARTITION OF orders ( + priority DEFAULT 10, + notes NOT NULL +) FOR VALUES IN ('us'); diff --git a/testdata/diff/create_table/issue_499_partition_column_overrides/new.sql b/testdata/diff/create_table/issue_499_partition_column_overrides/new.sql new file mode 100644 index 00000000..023cc207 --- /dev/null +++ b/testdata/diff/create_table/issue_499_partition_column_overrides/new.sql @@ -0,0 +1,14 @@ +CREATE TABLE public.orders ( + id bigint NOT NULL, + region text NOT NULL, + priority integer DEFAULT 0, + notes text +) PARTITION BY LIST (region); + +CREATE TABLE public.orders_eu PARTITION OF public.orders + FOR VALUES IN ('eu'); + +CREATE TABLE public.orders_us PARTITION OF public.orders ( + priority DEFAULT 10, + notes NOT NULL +) FOR VALUES IN ('us'); diff --git a/testdata/diff/create_table/issue_499_partition_column_overrides/old.sql b/testdata/diff/create_table/issue_499_partition_column_overrides/old.sql new file mode 100644 index 00000000..8e58a50f --- /dev/null +++ b/testdata/diff/create_table/issue_499_partition_column_overrides/old.sql @@ -0,0 +1,9 @@ +CREATE TABLE public.orders ( + id bigint NOT NULL, + region text NOT NULL, + priority integer DEFAULT 0, + notes text +) PARTITION BY LIST (region); + +CREATE TABLE public.orders_eu PARTITION OF public.orders + FOR VALUES IN ('eu'); diff --git a/testdata/diff/create_table/issue_499_partition_column_overrides/plan.json b/testdata/diff/create_table/issue_499_partition_column_overrides/plan.json new file mode 100644 index 00000000..c1e32bdd --- /dev/null +++ b/testdata/diff/create_table/issue_499_partition_column_overrides/plan.json @@ -0,0 +1,20 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.11.1", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "8872790ced32fbe419a3297dfdcd20a1b6e17650d2dcb1773161f505b66b2be8" + }, + "groups": [ + { + "steps": [ + { + "sql": "CREATE TABLE IF NOT EXISTS orders_us PARTITION OF orders (\n priority DEFAULT 10,\n notes NOT NULL\n) FOR VALUES IN ('us');", + "type": "table", + "operation": "create", + "path": "public.orders_us" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/issue_499_partition_column_overrides/plan.sql b/testdata/diff/create_table/issue_499_partition_column_overrides/plan.sql new file mode 100644 index 00000000..55f62f06 --- /dev/null +++ b/testdata/diff/create_table/issue_499_partition_column_overrides/plan.sql @@ -0,0 +1,4 @@ +CREATE TABLE IF NOT EXISTS orders_us PARTITION OF orders ( + priority DEFAULT 10, + notes NOT NULL +) FOR VALUES IN ('us'); diff --git a/testdata/diff/create_table/issue_499_partition_column_overrides/plan.txt b/testdata/diff/create_table/issue_499_partition_column_overrides/plan.txt new file mode 100644 index 00000000..9799ba0d --- /dev/null +++ b/testdata/diff/create_table/issue_499_partition_column_overrides/plan.txt @@ -0,0 +1,15 @@ +Plan: 1 to add. + +Summary by type: + tables: 1 to add + +Tables: + + orders_us + +DDL to be executed: +-------------------------------------------------- + +CREATE TABLE IF NOT EXISTS orders_us PARTITION OF orders ( + priority DEFAULT 10, + notes NOT NULL +) FOR VALUES IN ('us');