From 1c96d14435685c2317ca04ad052d7f9a87dafffa Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Mon, 12 Oct 2026 09:12:10 +0200 Subject: [PATCH 1/6] fix(dump): preserve partition index names and attachments Recursive parent-index creation generated default-named child indexes before dumps recreated explicitly named indexes. Replay therefore added redundant indexes, lost original attachments, and could change parent validity. Record parent-index identity in the IR. During export, create parent indexes with ON ONLY after tables and leaf indexes, then attach only the recorded children. Keep parent-index statements in ordered multi-file index files and defer foreign keys that require partitioned unique indexes. Leave migration index generation unchanged. Add single-file and multi-file replay regressions that compare index catalogs and plans against independently created sources. Cover custom names, unattached equivalent indexes, invalid parents, uniqueness, operator classes, formatting flags, and deterministic output. Validate on PostgreSQL 14 and 18 and retain the existing FK and dependency-order tests. --- cmd/dump/dump.go | 8 +- cmd/dump/partition_index_integration_test.go | 172 +++++++++++++++++++ cmd/plan/plan.go | 3 + cmd/plan/plan_test.go | 18 ++ docs/cli/dump.mdx | 7 + internal/diff/collector.go | 3 + internal/diff/diff.go | 19 +- internal/diff/index.go | 59 ++++--- internal/diff/partition_index.go | 98 +++++++++++ internal/diff/partition_index_test.go | 70 ++++++++ internal/diff/qualify_schema_test.go | 4 +- internal/diff/table.go | 10 +- internal/dump/formatter.go | 16 ++ ir/inspector.go | 2 + ir/ir.go | 2 + ir/queries/queries.sql | 7 + ir/queries/queries.sql.go | 11 ++ 17 files changed, 472 insertions(+), 37 deletions(-) create mode 100644 cmd/dump/partition_index_integration_test.go create mode 100644 internal/diff/partition_index.go create mode 100644 internal/diff/partition_index_test.go diff --git a/cmd/dump/dump.go b/cmd/dump/dump.go index cadc22bc..b3c7dccc 100644 --- a/cmd/dump/dump.go +++ b/cmd/dump/dump.go @@ -7,7 +7,6 @@ import ( "github.com/pgplex/pgschema/cmd/util" "github.com/pgplex/pgschema/internal/diff" "github.com/pgplex/pgschema/internal/dump" - "github.com/pgplex/pgschema/ir" "github.com/spf13/cobra" ) @@ -84,11 +83,8 @@ func ExecuteDump(config *DumpConfig) (string, error) { return "", fmt.Errorf("failed to get database schema: %w", err) } - // Create an empty schema for comparison to generate a dump diff - emptyIR := ir.NewIR() - - // Generate diff between empty schema and target schema (this represents a complete dump) - diffs := diff.GenerateMigrationWithOptions(emptyIR, schemaIR, config.Schema, config.QualifySchema) + // Generate the complete schema, preserving partitioned index identities. + diffs := diff.GenerateDump(schemaIR, config.Schema, config.QualifySchema) // Create dump formatter formatter := dump.NewDumpFormatter(schemaIR.Metadata.DatabaseVersion, config.Schema, config.NoComments, config.QualifySchema) diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go new file mode 100644 index 00000000..ccb04c43 --- /dev/null +++ b/cmd/dump/partition_index_integration_test.go @@ -0,0 +1,172 @@ +package dump + +import ( + "context" + "database/sql" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/pgplex/pgschema/cmd/plan" + "github.com/pgplex/pgschema/internal/include" + "github.com/pgplex/pgschema/testutil" +) + +// TestDumpCommand_PartitionIndexIdentity compares each dump with an independently +// created source, including deliberately unattached equivalent indexes. +func TestDumpCommand_PartitionIndexIdentity(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + source := testutil.SetupPostgres(t) + defer source.Stop() + conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, source) + defer conn.Close() + provider := testutil.SetupPostgres(t) + defer provider.Stop() + ctx := context.Background() + + const setup = ` +CREATE TABLE events (bucket int NOT NULL, id int NOT NULL, label text, payload text) PARTITION BY RANGE (bucket); +CREATE TABLE events_early PARTITION OF events FOR VALUES FROM (0) TO (10); +CREATE TABLE events_late PARTITION OF events FOR VALUES FROM (10) TO (20); +CREATE INDEX event_lookup ON ONLY events (label text_pattern_ops DESC NULLS LAST, id) INCLUDE (payload); +CREATE INDEX early_custom_lookup ON events_early (label text_pattern_ops DESC NULLS LAST, id) INCLUDE (payload); +CREATE INDEX late_custom_lookup ON events_late (label text_pattern_ops DESC NULLS LAST, id) INCLUDE (payload); +ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup; +ALTER INDEX event_lookup ATTACH PARTITION late_custom_lookup; +CREATE INDEX early_unattached_lookup ON events_early (label text_pattern_ops DESC NULLS LAST, id) INCLUDE (payload); +CREATE UNIQUE INDEX event_unique ON ONLY events (bucket, id); +CREATE UNIQUE INDEX early_custom_unique ON events_early (bucket, id); +CREATE UNIQUE INDEX late_custom_unique ON events_late (bucket, id); +ALTER INDEX event_unique ATTACH PARTITION early_custom_unique; +ALTER INDEX event_unique ATTACH PARTITION late_custom_unique; +CREATE TABLE event_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCES events (bucket, id)); +-- This parent intentionally remains invalid because only one child is attached. +CREATE INDEX event_partial ON ONLY events ((lower(label))) WHERE id > 0; +CREATE INDEX early_custom_partial ON events_early ((lower(label))) WHERE id > 0; +CREATE INDEX late_unattached_partial ON events_late ((lower(label))) WHERE id > 0; +ALTER INDEX event_partial ATTACH PARTITION early_custom_partial; +CREATE INDEX event_unattached_parent ON ONLY events (id); +CREATE INDEX early_unattached_id ON events_early (id); +CREATE INDEX "Event.Lookup" ON ONLY events (id DESC); +CREATE INDEX "Early.Lookup" ON events_early (id DESC); +CREATE INDEX "Late.Lookup" ON events_late (id DESC); +ALTER INDEX "Event.Lookup" ATTACH PARTITION "Early.Lookup"; +ALTER INDEX "Event.Lookup" ATTACH PARTITION "Late.Lookup"; +CREATE TABLE empty_events (id int) PARTITION BY RANGE (id); +CREATE INDEX empty_lookup ON empty_events (id); +COMMENT ON INDEX event_lookup IS 'Parent lookup'; +COMMENT ON INDEX early_custom_lookup IS 'Named child lookup'; +` + for _, multiFile := range []bool{false, true} { + for _, noComments := range []bool{false, true} { + for _, qualifySchema := range []bool{false, true} { + t.Run(fmt.Sprintf("multi=%v/noComments=%v/qualified=%v", multiFile, noComments, qualifySchema), func(t *testing.T) { + if _, err := conn.ExecContext(ctx, `DROP SCHEMA public CASCADE; CREATE SCHEMA public;`+setup); err != nil { + t.Fatal(err) + } + want := readIndexCatalog(t, conn) + wantFKs := foreignKeyCatalog(t, conn) + path := filepath.Join(t.TempDir(), "schema.sql") + config := &DumpConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", MultiFile: multiFile, File: path, + NoComments: noComments, QualifySchema: qualifySchema, + } + output, err := ExecuteDump(config) + if err != nil { + t.Fatal(err) + } + if !multiFile { + if err := os.WriteFile(path, []byte(output), 0600); err != nil { + t.Fatal(err) + } + } + // Compare the dump with the independent source BEFORE replay changes it. + migration, err := plan.GeneratePlan(&plan.PlanConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + }, provider) + if err != nil { + t.Errorf("Plan against independent source failed: %v", err) + } else if len(migration.Groups) != 0 { + t.Errorf("Expected empty plan against independent source, got: %+v", migration.Groups) + } + sqlText, err := include.NewProcessor(filepath.Dir(path)).ProcessFile(path) + if err != nil { + t.Fatal(err) + } + prefix := "" + if qualifySchema { + prefix = "public." + } + attachment := "ALTER INDEX " + prefix + "event_lookup ATTACH PARTITION " + prefix + "early_custom_lookup;" + if !strings.Contains(sqlText, attachment) { + t.Errorf("Missing explicit attachment: %s", attachment) + } + if strings.Contains(sqlText, "-- Name:") == noComments { + t.Error("Unexpected object header formatting") + } + mainContent, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + repeated, err := ExecuteDump(config) + if err != nil { + t.Fatal(err) + } + if multiFile { + repeatedMain, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(repeatedMain) != string(mainContent) { + t.Error("Non-deterministic include order or filenames") + } + repeated, err = include.NewProcessor(filepath.Dir(path)).ProcessFile(path) + if err != nil { + t.Fatal(err) + } + } + if repeated != sqlText { + t.Error("Non-deterministic SQL output") + } + if _, err := conn.ExecContext(ctx, `DROP SCHEMA public CASCADE; CREATE SCHEMA public;`+sqlText); err != nil { + t.Fatalf("Dump replay failed: %v", err) + } + if got := readIndexCatalog(t, conn); got != want { + t.Errorf("Index catalog changed after replay:\ngot: %s\nwant: %s", got, want) + } + if got := foreignKeyCatalog(t, conn); !reflect.DeepEqual(got, wantFKs) { + t.Errorf("Foreign keys changed after replay: got %v, want %v", got, wantFKs) + } + }) + } + } + } +} + +func readIndexCatalog(t *testing.T, conn *sql.DB) string { + t.Helper() + var catalog string + err := conn.QueryRowContext(context.Background(), ` +SELECT jsonb_agg(to_jsonb(x) ORDER BY table_name, name)::text FROM ( + SELECT t.relname AS table_name, c.relname AS name, c.relkind AS kind, + pg_get_indexdef(c.oid) AS definition, i.indisvalid AS valid, + i.indisready AS ready, i.indislive AS live, + obj_description(c.oid, 'pg_class') AS comment, + ARRAY(SELECT p.relname FROM pg_inherits h JOIN pg_class p ON p.oid = h.inhparent + WHERE h.inhrelid = c.oid ORDER BY h.inhseqno) AS parents + FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid + JOIN pg_class t ON t.oid = i.indrelid JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE n.nspname = 'public' +) x`).Scan(&catalog) + if err != nil { + t.Fatal(err) + } + return catalog +} diff --git a/cmd/plan/plan.go b/cmd/plan/plan.go index ea4dceb3..9125fcbf 100644 --- a/cmd/plan/plan.go +++ b/cmd/plan/plan.go @@ -584,6 +584,9 @@ func normalizeSchemaNames(irData *ir.IR, fromSchema, toSchema string) { if index.Schema == fromSchema { index.Schema = toSchema } + if index.ParentIndexSchema == fromSchema { + index.ParentIndexSchema = toSchema + } index.Where = stripQualifiers(replaceString(index.Where)) if index.IsExpression { for _, col := range index.Columns { diff --git a/cmd/plan/plan_test.go b/cmd/plan/plan_test.go index d5fa68e4..e9aa4613 100644 --- a/cmd/plan/plan_test.go +++ b/cmd/plan/plan_test.go @@ -289,3 +289,21 @@ func TestNormalizeSchemaNames_PreservesViewTableAliasesMatchingTargetSchema(t *t t.Fatalf("expected table alias to be preserved in view definition %q, got %q", expected, view.Definition) } } + +func TestNormalizeSchemaNames_IndexAttachment(t *testing.T) { + const temporary = "pgschema_tmp_index_attachment" + local := &ir.Index{Schema: temporary, ParentIndex: "parent", ParentIndexSchema: temporary} + external := &ir.Index{Schema: temporary, ParentIndex: "outside_parent", ParentIndexSchema: "other"} + data := &ir.IR{Schemas: map[string]*ir.Schema{ + temporary: {Name: temporary, Tables: map[string]*ir.Table{ + "child": {Schema: temporary, Name: "child", Indexes: map[string]*ir.Index{"local": local, "external": external}}, + }}, + }} + normalizeSchemaNames(data, temporary, "public") + if local.Schema != "public" || local.ParentIndexSchema != "public" || local.ParentIndex != "parent" { + t.Fatalf("Same-schema index attachment was not normalized: %+v", local) + } + if external.ParentIndexSchema != "other" || external.ParentIndex != "outside_parent" { + t.Fatalf("Cross-schema index attachment changed: %+v", external) + } +} diff --git a/docs/cli/dump.mdx b/docs/cli/dump.mdx index 7b5e18e2..207ea64d 100644 --- a/docs/cli/dump.mdx +++ b/docs/cli/dump.mdx @@ -256,6 +256,13 @@ prefix distinguishes multiple constraints on the same table. The exporter keeps these files in dependency order in `schema.sql`, after the required tables. Load files in the generated include order to preserve these dependencies. +For partitioned indexes, `pgschema` creates the tables and leaf indexes first. +The exporter then creates each parent index with `ON ONLY` and attaches its +recorded child indexes with `ALTER INDEX ... ATTACH PARTITION`. This preserves +custom child names and leaves intentionally unattached indexes separate. +Multi-file dumps put parent indexes and their attachments under `indexes/`, +for example `indexes/0001_events_lookup.sql`, after the table includes. + Each individual file (e.g., `tables/users.sql`) contains the specific object definition: ```sql -- diff --git a/internal/diff/collector.go b/internal/diff/collector.go index 68ceb90a..7bf165b1 100644 --- a/internal/diff/collector.go +++ b/internal/diff/collector.go @@ -18,6 +18,9 @@ type diffCollector struct { // for the target schema (set by `dump --qualify-schema`). Default false preserves // the standard "smart qualification" (omit the target-schema prefix). qualifySchema bool + + // dump preserves explicit partitioned index creation and attachment order. + dump bool } // newDiffCollector creates a new diffCollector diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 64112bb4..ed19f770 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -484,7 +484,7 @@ type rlsChange struct { // GenerateMigration generates the migration diff using standard "smart qualification" // (the target-schema prefix is omitted on entity names). func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { - return generateMigration(oldIR, newIR, targetSchema, false, 0) + return generateMigration(oldIR, newIR, targetSchema, false, 0, false) } // GenerateMigrationForTarget is GenerateMigration for a target database whose @@ -493,7 +493,7 @@ func GenerateMigration(oldIR, newIR *ir.IR, targetSchema string) []Diff { // ALTER COLUMN ... SET EXPRESSION AS (PostgreSQL 17+); older targets get a // version-portable equivalent instead (issue #591). func GenerateMigrationForTarget(oldIR, newIR *ir.IR, targetSchema string, targetMajorVersion int) []Diff { - return generateMigration(oldIR, newIR, targetSchema, false, targetMajorVersion) + return generateMigration(oldIR, newIR, targetSchema, false, targetMajorVersion, false) } // GenerateMigrationWithOptions is like GenerateMigration, but when qualifySchema is @@ -506,10 +506,16 @@ func GenerateMigrationForTarget(oldIR, newIR *ir.IR, targetSchema string, target // stay bare because the IR stores them without schema identity (#493). Default // behavior (false) is unchanged for plan/apply. func GenerateMigrationWithOptions(oldIR, newIR *ir.IR, targetSchema string, qualifySchema bool) []Diff { - return generateMigration(oldIR, newIR, targetSchema, qualifySchema, 0) + return generateMigration(oldIR, newIR, targetSchema, qualifySchema, 0, false) } -func generateMigration(oldIR, newIR *ir.IR, targetSchema string, qualifySchema bool, targetMajorVersion int) []Diff { +// GenerateDump creates an empty-to-target export while preserving partitioned +// index identities. Migration entry points retain recursive index creation. +func GenerateDump(schema *ir.IR, targetSchema string, qualifySchema bool) []Diff { + return generateMigration(ir.NewIR(), schema, targetSchema, qualifySchema, 0, true) +} + +func generateMigration(oldIR, newIR *ir.IR, targetSchema string, qualifySchema bool, targetMajorVersion int, dump bool) []Diff { diff := &ddlDiff{ targetMajorVersion: targetMajorVersion, addedSchemas: []*ir.Schema{}, @@ -1634,6 +1640,7 @@ func generateMigration(oldIR, newIR *ir.IR, targetSchema string, qualifySchema b // Create a diffCollector and generate SQL collector := newDiffCollector() collector.qualifySchema = qualifySchema + collector.dump = dump diff.collectMigrationSQL(targetSchema, collector) return collector.diffs } @@ -2136,6 +2143,10 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateSequenceComment(seq, targetSchema, DiffOperationCreate, collector) } + if collector.dump { + generatePartitionedIndexesSQL(d.addedTables, targetSchema, collector) + } + // Add deferred foreign key constraints from ALL batches AFTER all tables are created // This ensures FK references to tables in any batch work correctly allDeferredConstraints := append(append(deferredConstraints1, deferredConstraints2...), deferredConstraints3...) diff --git a/internal/diff/index.go b/internal/diff/index.go index ca98b83c..d450eeb6 100644 --- a/internal/diff/index.go +++ b/internal/diff/index.go @@ -28,35 +28,44 @@ func generateCreateIndexesSQLWithType(indexes []*ir.Index, targetSchema string, continue } - canonicalSQL := generateIndexSQLMode(index, targetSchema, false, collector.qualifySchema) // Always generate canonical form + if collector.dump && index.IsPartitioned { + // Export these after every table exists. Creating a partition after + // its parent's index would make PostgreSQL generate child indexes. + continue + } + generateCreateIndexSQL(index, targetSchema, collector, indexType, commentType, false) + } +} + +func generateCreateIndexSQL(index *ir.Index, targetSchema string, collector *diffCollector, indexType DiffType, commentType DiffType, only bool) { + canonicalSQL := generateIndexSQLWithOptions(index, index.Name, targetSchema, false, collector.qualifySchema, only) + + // Create context for this statement + context := &diffContext{ + Type: indexType, + Operation: DiffOperationCreate, + Path: fmt.Sprintf("%s.%s.%s", index.Schema, index.Table, index.Name), + Source: index, + CanRunInTransaction: true, + } + + collector.collect(context, canonicalSQL) + + // Add index comment + if index.Comment != "" { + indexName := qualifyEntityNameMode(index.Schema, index.Name, targetSchema, collector.qualifySchema) + sql := fmt.Sprintf("COMMENT ON INDEX %s IS %s;", indexName, quoteString(index.Comment)) // Create context for this statement context := &diffContext{ - Type: indexType, + Type: commentType, Operation: DiffOperationCreate, Path: fmt.Sprintf("%s.%s.%s", index.Schema, index.Table, index.Name), Source: index, - CanRunInTransaction: true, + CanRunInTransaction: true, // Comments can always run in a transaction } - collector.collect(context, canonicalSQL) - - // Add index comment - if index.Comment != "" { - indexName := qualifyEntityNameMode(index.Schema, index.Name, targetSchema, collector.qualifySchema) - sql := fmt.Sprintf("COMMENT ON INDEX %s IS %s;", indexName, quoteString(index.Comment)) - - // Create context for this statement - context := &diffContext{ - Type: commentType, - Operation: DiffOperationCreate, - Path: fmt.Sprintf("%s.%s.%s", index.Schema, index.Table, index.Name), - Source: index, - CanRunInTransaction: true, // Comments can always run in a transaction - } - - collector.collect(context, sql) - } + collector.collect(context, sql) } } @@ -79,6 +88,10 @@ func generateIndexSQLWithName(index *ir.Index, indexName string, targetSchema st // generateIndexSQLWithNameMode is like generateIndexSQLWithName, but when qualifySchema // is true the ON-table reference is always schema-qualified, even within the target schema. func generateIndexSQLWithNameMode(index *ir.Index, indexName string, targetSchema string, isConcurrent bool, qualifySchema bool) string { + return generateIndexSQLWithOptions(index, indexName, targetSchema, isConcurrent, qualifySchema, false) +} + +func generateIndexSQLWithOptions(index *ir.Index, indexName string, targetSchema string, isConcurrent bool, qualifySchema bool, only bool) string { var builder strings.Builder // CREATE [UNIQUE] INDEX [CONCURRENTLY] IF NOT EXISTS @@ -96,6 +109,10 @@ func generateIndexSQLWithNameMode(index *ir.Index, indexName string, targetSchem builder.WriteString(ir.QuoteIdentifier(indexName)) builder.WriteString(" ON ") + if only { + builder.WriteString("ONLY ") + } + // Table name with proper schema qualification tableName := getTableNameWithSchemaMode(index.Schema, index.Table, targetSchema, qualifySchema) builder.WriteString(tableName) diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go new file mode 100644 index 00000000..ded87607 --- /dev/null +++ b/internal/diff/partition_index.go @@ -0,0 +1,98 @@ +package diff + +import ( + "fmt" + "sort" + + "github.com/pgplex/pgschema/ir" +) + +// generatePartitionedIndexesSQL emits parent indexes after all partition tables +// and leaf indexes exist. ON ONLY prevents PostgreSQL from creating or choosing +// child indexes implicitly. Explicit attachments preserve names and validity. +func generatePartitionedIndexesSQL(tables []*ir.Table, targetSchema string, collector *diffCollector) { + type indexKey struct{ schema, name string } + parents := make(map[indexKey]*ir.Index) + children := make(map[indexKey][]*ir.Index) + var keys []indexKey + for _, table := range tables { + for _, index := range table.Indexes { + if index.Type == ir.IndexTypePrimary { + continue // Primary indexes are created by constraints. + } + key := indexKey{index.Schema, index.Name} + if index.IsPartitioned { + parents[key] = index + keys = append(keys, key) + } + if index.ParentIndex != "" { + parent := indexKey{index.ParentIndexSchema, index.ParentIndex} + children[parent] = append(children[parent], index) + } + } + } + sort.Slice(keys, func(i, j int) bool { + if keys[i].schema != keys[j].schema { + return keys[i].schema < keys[j].schema + } + return keys[i].name < keys[j].name + }) + created := make(map[indexKey]bool) + var create func(indexKey) + create = func(key indexKey) { + if created[key] { + return + } + created[key] = true + attached := children[key] + sort.Slice(attached, func(i, j int) bool { + if attached[i].Schema != attached[j].Schema { + return attached[i].Schema < attached[j].Schema + } + return attached[i].Name < attached[j].Name + }) + // A partitioned child must exist before its own attachment is emitted. + for _, child := range attached { + if child.IsPartitioned { + create(indexKey{child.Schema, child.Name}) + } + } + index := parents[key] + generateCreateIndexSQL(index, targetSchema, collector, DiffTypeTableIndex, DiffTypeTableIndexComment, true) + for _, child := range attached { + sql := fmt.Sprintf("ALTER INDEX %s ATTACH PARTITION %s;", + qualifyEntityNameMode(index.Schema, index.Name, targetSchema, collector.qualifySchema), + qualifyEntityNameMode(child.Schema, child.Name, targetSchema, collector.qualifySchema)) + collector.collect(&diffContext{ + Type: DiffTypeTableIndex, Operation: DiffOperationAlter, + Path: fmt.Sprintf("%s.%s.%s", index.Schema, index.Table, index.Name), + Source: index, CanRunInTransaction: true, + }, sql) + } + } + for _, key := range keys { + create(key) + } +} + +// referencesPartitionedUniqueIndex identifies FKs that may need a postponed +// unique index. Those FKs must wait until the index and its attachments exist. +func referencesPartitionedUniqueIndex(table *ir.Table, constraint *ir.Constraint, tables map[string]*ir.Table) bool { + if constraint == nil || constraint.Type != ir.ConstraintTypeForeignKey { + return false + } + schema := constraint.ReferencedSchema + if schema == "" { + schema = table.Schema + } + parent := tables[schema+"."+constraint.ReferencedTable] + if parent == nil || !parent.IsPartitioned { + return false + } + for _, index := range parent.Indexes { + if index.Type == ir.IndexTypeUnique { + return true + } + } + return false +} diff --git a/internal/diff/partition_index_test.go b/internal/diff/partition_index_test.go new file mode 100644 index 00000000..636408c1 --- /dev/null +++ b/internal/diff/partition_index_test.go @@ -0,0 +1,70 @@ +package diff + +import ( + "strings" + "testing" + + "github.com/pgplex/pgschema/ir" +) + +// TestGenerateDumpPartitionIndexOrder keeps the export-only index strategy out +// of migration generation and checks ordering independently of the formatter. +func TestGenerateDumpPartitionIndexOrder(t *testing.T) { + parent := &ir.Index{ + Schema: "public", Table: "events", Name: "parent_lookup", + Type: ir.IndexTypeRegular, Method: "btree", IsPartitioned: true, + Columns: []*ir.IndexColumn{{Name: "id", Position: 1}}, + } + child := &ir.Index{ + Schema: "public", Table: "events_early", Name: "custom_lookup", + Type: ir.IndexTypeRegular, Method: "btree", + Columns: []*ir.IndexColumn{{Name: "id", Position: 1}}, + ParentIndex: parent.Name, ParentIndexSchema: parent.Schema, + } + schema := ir.NewIR() + schema.Schemas["public"] = &ir.Schema{Name: "public", Tables: map[string]*ir.Table{ + "events": { + Schema: "public", Name: "events", IsPartitioned: true, + PartitionStrategy: "RANGE", PartitionKey: "id", + Columns: []*ir.Column{{Name: "id", Position: 1, DataType: "integer"}}, + Indexes: map[string]*ir.Index{parent.Name: parent}, + }, + "events_early": { + Schema: "public", Name: "events_early", PartitionOf: "events", PartitionOfSchema: "public", + PartitionBound: "FOR VALUES FROM (0) TO (10)", + Columns: []*ir.Column{{Name: "id", Position: 1, DataType: "integer"}}, + Indexes: map[string]*ir.Index{child.Name: child}, + }, + }} + wantOrder := []string{ + "CREATE TABLE IF NOT EXISTS events (", + "CREATE TABLE IF NOT EXISTS events_early PARTITION OF events", + "CREATE INDEX IF NOT EXISTS custom_lookup ON events_early (id);", + "CREATE INDEX IF NOT EXISTS parent_lookup ON ONLY events (id);", + "ALTER INDEX parent_lookup ATTACH PARTITION custom_lookup;", + } + var first string + for run := 0; run < 5; run++ { + sql := buildSQLFromSteps(GenerateDump(schema, "public", false)) + previous := -1 + for _, stmt := range wantOrder { + position := strings.Index(sql, stmt) + if position <= previous { + t.Fatalf("Missing or misplaced statement %q:\n%s", stmt, sql) + } + previous = position + } + if run == 0 { + first = sql + } else if sql != first { + t.Fatal("Non-deterministic partitioned index export") + } + } + migration := buildSQLFromSteps(GenerateMigration(ir.NewIR(), schema, "public")) + if strings.Contains(migration, "ON ONLY") || strings.Contains(migration, "ATTACH PARTITION") { + t.Fatalf("Dump-only index strategy changed migration generation:\n%s", migration) + } + if !strings.Contains(migration, "CREATE INDEX IF NOT EXISTS parent_lookup ON events (id);") { + t.Fatalf("Expected existing recursive migration strategy:\n%s", migration) + } +} diff --git a/internal/diff/qualify_schema_test.go b/internal/diff/qualify_schema_test.go index 4d120d9e..d1d44c8c 100644 --- a/internal/diff/qualify_schema_test.go +++ b/internal/diff/qualify_schema_test.go @@ -60,7 +60,7 @@ func TestQualifySchema_TableAndColumnType(t *testing.T) { } empty := map[string]bool{} - def, _ := generateTableSQL(table, "public", false, empty, empty, empty, nil) + def, _ := generateTableSQL(table, "public", false, empty, empty, empty, nil, false) if !strings.Contains(def, "CREATE TABLE IF NOT EXISTS account (") { t.Errorf("default should use the bare table name: %q", def) } @@ -71,7 +71,7 @@ func TestQualifySchema_TableAndColumnType(t *testing.T) { t.Errorf("default should strip the target-schema prefix from the type ref: %q", def) } - qualified, _ := generateTableSQL(table, "public", true, empty, empty, empty, nil) + qualified, _ := generateTableSQL(table, "public", true, empty, empty, empty, nil, false) 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 c5571876..2d862f77 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -495,7 +495,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, allNewTables) + sql, tableDeferred := generateTableSQL(table, targetSchema, collector.qualifySchema, createdTables, existingTables, suppressedInlineFKs, allNewTables, collector.dump) deferredConstraints = append(deferredConstraints, tableDeferred...) // Create context for this statement @@ -917,7 +917,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, allTables map[string]*ir.Table) (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, dump bool) (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) @@ -982,7 +982,8 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, if suppressedInlineFKs[constraintPathKey(constraint)] { continue } - if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) { + if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) || + (dump && referencesPartitionedUniqueIndex(table, constraint, allTables)) { deferred = append(deferred, &deferredConstraint{ table: table, constraint: constraint, @@ -1046,7 +1047,8 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, if suppressedInlineFKs[constraintPathKey(constraint)] { continue } - if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) { + if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) || + (dump && referencesPartitionedUniqueIndex(table, constraint, allTables)) { deferred = append(deferred, &deferredConstraint{ table: table, constraint: constraint, diff --git a/internal/dump/formatter.go b/internal/dump/formatter.go index ce7d1169..23637a8c 100644 --- a/internal/dump/formatter.go +++ b/internal/dump/formatter.go @@ -92,6 +92,8 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er stepsByFile := make(map[fileKey][]diff.Diff) var fileOrder []fileKey constraintIndex := 0 + type indexKey struct{ schema, name string } + partitionIndexFiles := make(map[indexKey]fileKey) var tables []*ir.Table var views []*ir.View @@ -113,6 +115,20 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er key = fileKey{dir: "constraints", name: fmt.Sprintf("%04d_%s", constraintIndex, key.name)} } + if index, ok := step.Source.(*ir.Index); ok && index.IsPartitioned && + (step.Type == diff.DiffTypeTableIndex || step.Type == diff.DiffTypeTableIndexComment) { + // Parent indexes and their attachments follow all partition tables. + // Keep each parent's statements together without moving them back + // into the table file. Numbering avoids sanitized-name collisions. + identity := indexKey{index.Schema, index.Name} + if existing, exists := partitionIndexFiles[identity]; exists { + key = existing + } else { + key = fileKey{dir: "indexes", name: fmt.Sprintf("%04d_%s", len(partitionIndexFiles)+1, index.Name)} + partitionIndexFiles[identity] = key + } + } + if _, exists := stepsByFile[key]; !exists { fileOrder = append(fileOrder, key) } diff --git a/ir/inspector.go b/ir/inspector.go index 39e42e6a..78733c7f 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -855,6 +855,8 @@ func (i *Inspector) buildIndexes(ctx context.Context, schema *IR, targetSchema s IsExpression: hasExpressions, Where: "", StorageParameters: indexRow.Reloptions, + ParentIndex: indexRow.ParentIndex.String, + ParentIndexSchema: indexRow.ParentIndexSchema.String, Comment: comment, Columns: []*IndexColumn{}, } diff --git a/ir/ir.go b/ir/ir.go index ba2db031..7b71f6a5 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -296,6 +296,8 @@ type Index struct { Where string `json:"where,omitempty"` // partial index condition NullsNotDistinct bool `json:"nulls_not_distinct,omitempty"` // NULLS NOT DISTINCT (PG15+) IsPartitioned bool `json:"is_partitioned,omitempty"` // index target is a partitioned parent (relkind 'p') + ParentIndex string `json:"parent_index,omitempty"` // attached parent index, empty for unattached indexes + ParentIndexSchema string `json:"parent_index_schema,omitempty"` StorageParameters []string `json:"storage_parameters,omitempty"` // WITH (fillfactor=100, ...) Comment string `json:"comment,omitempty"` } diff --git a/ir/queries/queries.sql b/ir/queries/queries.sql index 93a31636..60ec9800 100644 --- a/ir/queries/queries.sql +++ b/ir/queries/queries.sql @@ -564,6 +564,8 @@ WITH index_base AS ( n.nspname as schemaname, t.relname as tablename, i.relname as indexname, + pi.relname AS parent_index, + pn.nspname AS parent_index_schema, idx.indisunique as is_unique, idx.indisprimary as is_primary, (idx.indpred IS NOT NULL) as is_partial, @@ -617,6 +619,9 @@ WITH index_base AS ( JOIN pg_namespace n ON n.oid = t.relnamespace JOIN pg_am am ON am.oid = i.relam LEFT JOIN pg_description d ON d.objoid = i.oid AND d.objsubid = 0 + LEFT JOIN pg_inherits inh ON inh.inhrelid = i.oid + LEFT JOIN pg_class pi ON pi.oid = inh.inhparent + LEFT JOIN pg_namespace pn ON pn.oid = pi.relnamespace WHERE NOT idx.indisprimary AND NOT EXISTS ( @@ -648,6 +653,8 @@ SELECT ib.schemaname, ib.tablename, ib.indexname, + ib.parent_index, + ib.parent_index_schema, ib.is_unique, ib.is_primary, ib.is_partial, diff --git a/ir/queries/queries.sql.go b/ir/queries/queries.sql.go index c581f11c..2dfc73e3 100644 --- a/ir/queries/queries.sql.go +++ b/ir/queries/queries.sql.go @@ -2196,6 +2196,8 @@ WITH index_base AS ( n.nspname as schemaname, t.relname as tablename, i.relname as indexname, + pi.relname AS parent_index, + pn.nspname AS parent_index_schema, idx.indisunique as is_unique, idx.indisprimary as is_primary, (idx.indpred IS NOT NULL) as is_partial, @@ -2249,6 +2251,9 @@ WITH index_base AS ( JOIN pg_namespace n ON n.oid = t.relnamespace JOIN pg_am am ON am.oid = i.relam LEFT JOIN pg_description d ON d.objoid = i.oid AND d.objsubid = 0 + LEFT JOIN pg_inherits inh ON inh.inhrelid = i.oid + LEFT JOIN pg_class pi ON pi.oid = inh.inhparent + LEFT JOIN pg_namespace pn ON pn.oid = pi.relnamespace WHERE NOT idx.indisprimary AND NOT EXISTS ( @@ -2280,6 +2285,8 @@ SELECT ib.schemaname, ib.tablename, ib.indexname, + ib.parent_index, + ib.parent_index_schema, ib.is_unique, ib.is_primary, ib.is_partial, @@ -2316,6 +2323,8 @@ type GetIndexesForSchemaRow struct { Schemaname string `db:"schemaname" json:"schemaname"` Tablename string `db:"tablename" json:"tablename"` Indexname string `db:"indexname" json:"indexname"` + ParentIndex sql.NullString `db:"parent_index" json:"parent_index"` + ParentIndexSchema sql.NullString `db:"parent_index_schema" json:"parent_index_schema"` IsUnique bool `db:"is_unique" json:"is_unique"` IsPrimary bool `db:"is_primary" json:"is_primary"` IsPartial sql.NullBool `db:"is_partial" json:"is_partial"` @@ -2351,6 +2360,8 @@ func (q *Queries) GetIndexesForSchema(ctx context.Context, dollar_1 sql.NullStri &i.Schemaname, &i.Tablename, &i.Indexname, + &i.ParentIndex, + &i.ParentIndexSchema, &i.IsUnique, &i.IsPrimary, &i.IsPartial, From 33949091120babd730b5f2d25cea2bc99731c6ba Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Mon, 12 Oct 2026 21:29:28 +0200 Subject: [PATCH 2/6] fix(diff): preserve partition keys on nested partitions --- cmd/dump/partition_index_integration_test.go | 13 +++++++ internal/diff/table.go | 18 ++++----- .../create_table/nested_partitions/diff.sql | 12 ++++++ .../create_table/nested_partitions/new.sql | 11 ++++++ .../create_table/nested_partitions/old.sql | 1 + .../create_table/nested_partitions/plan.json | 38 +++++++++++++++++++ .../create_table/nested_partitions/plan.sql | 12 ++++++ .../create_table/nested_partitions/plan.txt | 26 +++++++++++++ 8 files changed, 122 insertions(+), 9 deletions(-) create mode 100644 testdata/diff/create_table/nested_partitions/diff.sql create mode 100644 testdata/diff/create_table/nested_partitions/new.sql create mode 100644 testdata/diff/create_table/nested_partitions/old.sql create mode 100644 testdata/diff/create_table/nested_partitions/plan.json create mode 100644 testdata/diff/create_table/nested_partitions/plan.sql create mode 100644 testdata/diff/create_table/nested_partitions/plan.txt diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go index ccb04c43..dec139f5 100644 --- a/cmd/dump/partition_index_integration_test.go +++ b/cmd/dump/partition_index_integration_test.go @@ -59,6 +59,19 @@ ALTER INDEX "Event.Lookup" ATTACH PARTITION "Early.Lookup"; ALTER INDEX "Event.Lookup" ATTACH PARTITION "Late.Lookup"; CREATE TABLE empty_events (id int) PARTITION BY RANGE (id); CREATE INDEX empty_lookup ON empty_events (id); +-- The root index sorts before its partitioned child to exercise recursive ordering. +CREATE TABLE archive_events (bucket int, label text) PARTITION BY RANGE (bucket); +CREATE TABLE archive_events_early PARTITION OF archive_events FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); +CREATE TABLE archive_events_first PARTITION OF archive_events_early FOR VALUES FROM (0) TO (5); +CREATE TABLE archive_events_second PARTITION OF archive_events_early FOR VALUES FROM (5) TO (10); +CREATE INDEX a_archive_lookup ON ONLY archive_events (label); +CREATE INDEX z_archive_lookup ON ONLY archive_events_early (label); +CREATE INDEX first_custom_lookup ON archive_events_first (label); +CREATE INDEX second_custom_lookup ON archive_events_second (label); +ALTER INDEX z_archive_lookup ATTACH PARTITION first_custom_lookup; +ALTER INDEX z_archive_lookup ATTACH PARTITION second_custom_lookup; +ALTER INDEX a_archive_lookup ATTACH PARTITION z_archive_lookup; +CREATE INDEX first_unattached_lookup ON archive_events_first (label); COMMENT ON INDEX event_lookup IS 'Parent lookup'; COMMENT ON INDEX early_custom_lookup IS 'Named child lookup'; ` diff --git a/internal/diff/table.go b/internal/diff/table.go index 2d862f77..2a3f5405 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -922,6 +922,11 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, // forced qualification is on). tableName := ir.QualifyEntityNameWithQuotesMode(table.Schema, table.Name, targetSchema, qualifySchema) + partitionClause := "" + if table.IsPartitioned && table.PartitionStrategy != "" && table.PartitionKey != "" { + partitionClause = fmt.Sprintf(" PARTITION BY %s (%s)", table.PartitionStrategy, table.PartitionKey) + } + // Partition children: emit PARTITION OF instead of standalone CREATE TABLE. // Columns and inherited constraints come from the parent automatically; // only child-specific constraints (conparentid = 0) remain in the IR. @@ -1003,10 +1008,10 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, allParts := append(elementParts, constraintParts...) var sql string if len(allParts) > 0 { - sql = fmt.Sprintf("%s %s PARTITION OF %s (\n%s\n) %s;", - createPrefix, tableName, parentName, strings.Join(allParts, ",\n"), table.PartitionBound) + sql = fmt.Sprintf("%s %s PARTITION OF %s (\n%s\n) %s%s;", + createPrefix, tableName, parentName, strings.Join(allParts, ",\n"), table.PartitionBound, partitionClause) } else { - sql = fmt.Sprintf("%s %s PARTITION OF %s %s;", createPrefix, tableName, parentName, table.PartitionBound) + sql = fmt.Sprintf("%s %s PARTITION OF %s %s%s;", createPrefix, tableName, parentName, table.PartitionBound, partitionClause) } return sql, deferred } @@ -1063,12 +1068,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, parts = append(parts, strings.Join(columnParts, ",\n")) - // Add partition clause for partitioned tables - if table.IsPartitioned && table.PartitionStrategy != "" && table.PartitionKey != "" { - parts = append(parts, fmt.Sprintf(") PARTITION BY %s (%s);", table.PartitionStrategy, table.PartitionKey)) - } else { - parts = append(parts, ");") - } + parts = append(parts, ")"+partitionClause+";") return strings.Join(parts, "\n"), deferred } diff --git a/testdata/diff/create_table/nested_partitions/diff.sql b/testdata/diff/create_table/nested_partitions/diff.sql new file mode 100644 index 00000000..30c796f8 --- /dev/null +++ b/testdata/diff/create_table/nested_partitions/diff.sql @@ -0,0 +1,12 @@ +CREATE TABLE IF NOT EXISTS events ( + bucket integer NOT NULL, + label text +) PARTITION BY RANGE (bucket); + +CREATE TABLE IF NOT EXISTS events_early PARTITION OF events ( + label DEFAULT 'pending' +) FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); + +CREATE TABLE IF NOT EXISTS events_first PARTITION OF events_early FOR VALUES FROM (0) TO (5); + +CREATE TABLE IF NOT EXISTS events_second PARTITION OF events_early FOR VALUES FROM (5) TO (10); diff --git a/testdata/diff/create_table/nested_partitions/new.sql b/testdata/diff/create_table/nested_partitions/new.sql new file mode 100644 index 00000000..61fdde91 --- /dev/null +++ b/testdata/diff/create_table/nested_partitions/new.sql @@ -0,0 +1,11 @@ +CREATE TABLE public.events ( + bucket integer NOT NULL, + label text +) PARTITION BY RANGE (bucket); + +CREATE TABLE public.events_early PARTITION OF public.events ( + label DEFAULT 'pending' +) FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); + +CREATE TABLE public.events_first PARTITION OF public.events_early FOR VALUES FROM (0) TO (5); +CREATE TABLE public.events_second PARTITION OF public.events_early FOR VALUES FROM (5) TO (10); diff --git a/testdata/diff/create_table/nested_partitions/old.sql b/testdata/diff/create_table/nested_partitions/old.sql new file mode 100644 index 00000000..03d7e9ae --- /dev/null +++ b/testdata/diff/create_table/nested_partitions/old.sql @@ -0,0 +1 @@ +-- Empty schema diff --git a/testdata/diff/create_table/nested_partitions/plan.json b/testdata/diff/create_table/nested_partitions/plan.json new file mode 100644 index 00000000..277b6527 --- /dev/null +++ b/testdata/diff/create_table/nested_partitions/plan.json @@ -0,0 +1,38 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.13.1", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "965b1131737c955e24c7f827c55bd78e4cb49a75adfd04229e0ba297376f5085" + }, + "groups": [ + { + "steps": [ + { + "sql": "CREATE TABLE IF NOT EXISTS events (\n bucket integer NOT NULL,\n label text\n) PARTITION BY RANGE (bucket);", + "type": "table", + "operation": "create", + "path": "public.events" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_early PARTITION OF events (\n label DEFAULT 'pending'\n) FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket);", + "type": "table", + "operation": "create", + "path": "public.events_early" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_first PARTITION OF events_early FOR VALUES FROM (0) TO (5);", + "type": "table", + "operation": "create", + "path": "public.events_first" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_second PARTITION OF events_early FOR VALUES FROM (5) TO (10);", + "type": "table", + "operation": "create", + "path": "public.events_second" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/nested_partitions/plan.sql b/testdata/diff/create_table/nested_partitions/plan.sql new file mode 100644 index 00000000..30c796f8 --- /dev/null +++ b/testdata/diff/create_table/nested_partitions/plan.sql @@ -0,0 +1,12 @@ +CREATE TABLE IF NOT EXISTS events ( + bucket integer NOT NULL, + label text +) PARTITION BY RANGE (bucket); + +CREATE TABLE IF NOT EXISTS events_early PARTITION OF events ( + label DEFAULT 'pending' +) FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); + +CREATE TABLE IF NOT EXISTS events_first PARTITION OF events_early FOR VALUES FROM (0) TO (5); + +CREATE TABLE IF NOT EXISTS events_second PARTITION OF events_early FOR VALUES FROM (5) TO (10); diff --git a/testdata/diff/create_table/nested_partitions/plan.txt b/testdata/diff/create_table/nested_partitions/plan.txt new file mode 100644 index 00000000..2608c66b --- /dev/null +++ b/testdata/diff/create_table/nested_partitions/plan.txt @@ -0,0 +1,26 @@ +Plan: 4 to add. + +Summary by type: + tables: 4 to add + +Tables: + + events + + events_early + + events_first + + events_second + +DDL to be executed: +-------------------------------------------------- + +CREATE TABLE IF NOT EXISTS events ( + bucket integer NOT NULL, + label text +) PARTITION BY RANGE (bucket); + +CREATE TABLE IF NOT EXISTS events_early PARTITION OF events ( + label DEFAULT 'pending' +) FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); + +CREATE TABLE IF NOT EXISTS events_first PARTITION OF events_early FOR VALUES FROM (0) TO (5); + +CREATE TABLE IF NOT EXISTS events_second PARTITION OF events_early FOR VALUES FROM (5) TO (10); From f1d40f5541af32acba81b3382719928651f1bf43 Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Mon, 12 Oct 2026 22:15:48 +0200 Subject: [PATCH 3/6] fix(dump): reject ignored partition index dependencies --- cmd/dump/dump.go | 5 +- cmd/dump/partition_index_ignore_test.go | 135 ++++++++++++++++++++++++ docs/cli/ignore.mdx | 16 +++ internal/diff/diff.go | 7 +- internal/diff/partition_index.go | 32 ++++++ internal/diff/partition_index_test.go | 51 ++++++++- ir/index_test.go | 22 ++++ ir/inspector.go | 38 ++++--- ir/ir.go | 10 ++ 9 files changed, 297 insertions(+), 19 deletions(-) create mode 100644 cmd/dump/partition_index_ignore_test.go create mode 100644 ir/index_test.go diff --git a/cmd/dump/dump.go b/cmd/dump/dump.go index b3c7dccc..281e748b 100644 --- a/cmd/dump/dump.go +++ b/cmd/dump/dump.go @@ -84,7 +84,10 @@ func ExecuteDump(config *DumpConfig) (string, error) { } // Generate the complete schema, preserving partitioned index identities. - diffs := diff.GenerateDump(schemaIR, config.Schema, config.QualifySchema) + diffs, err := diff.GenerateDump(schemaIR, config.Schema, config.QualifySchema) + if err != nil { + return "", fmt.Errorf("failed to generate dump: %w", err) + } // Create dump formatter formatter := dump.NewDumpFormatter(schemaIR.Metadata.DatabaseVersion, config.Schema, config.NoComments, config.QualifySchema) diff --git a/cmd/dump/partition_index_ignore_test.go b/cmd/dump/partition_index_ignore_test.go new file mode 100644 index 00000000..73869ceb --- /dev/null +++ b/cmd/dump/partition_index_ignore_test.go @@ -0,0 +1,135 @@ +package dump + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pgplex/pgschema/internal/include" + "github.com/pgplex/pgschema/testutil" +) + +func TestDumpCommand_IgnoredPartitionIndexes(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + source := testutil.SetupPostgres(t) + defer source.Stop() + conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, source) + defer conn.Close() + + const setup = ` +CREATE TABLE events (bucket int, id int) PARTITION BY RANGE (bucket); +CREATE TABLE events_early PARTITION OF events FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (id); +CREATE TABLE events_late PARTITION OF events FOR VALUES FROM (10) TO (20); +CREATE TABLE events_first PARTITION OF events_early FOR VALUES FROM (0) TO (10); +CREATE TABLE events_second PARTITION OF events_early FOR VALUES FROM (10) TO (20); +CREATE UNIQUE INDEX event_unique ON ONLY events (bucket, id); +CREATE UNIQUE INDEX early_custom_unique ON ONLY events_early (bucket, id); +CREATE UNIQUE INDEX late_custom_unique ON events_late (bucket, id); +CREATE UNIQUE INDEX first_custom_unique ON events_first (bucket, id); +CREATE UNIQUE INDEX second_custom_unique ON events_second (bucket, id); +ALTER INDEX early_custom_unique ATTACH PARTITION first_custom_unique; +ALTER INDEX early_custom_unique ATTACH PARTITION second_custom_unique; +ALTER INDEX event_unique ATTACH PARTITION early_custom_unique; +ALTER INDEX event_unique ATTACH PARTITION late_custom_unique; +CREATE TABLE event_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCES events (bucket, id)); +CREATE INDEX unattached_lookup ON events_first (id); +CREATE INDEX partial_lookup ON ONLY events (id); +CREATE INDEX late_partial_lookup ON events_late (id); +ALTER INDEX partial_lookup ATTACH PARTITION late_partial_lookup; +` + for _, tc := range []struct { + name string + ignore string + parent string + child string + dropFK bool + absent string + }{ + {name: "complete_family"}, + {name: "ignored_leaf", ignore: "[indexes]\npatterns = [\"late_custom_unique\"]\n", parent: "event_unique", child: "late_custom_unique"}, + {name: "ignored_intermediate", ignore: "[indexes]\npatterns = [\"early_custom_unique\"]\n", parent: "event_unique", child: "early_custom_unique"}, + {name: "ignored_nested_leaf", ignore: "[indexes]\npatterns = [\"first_custom_unique\"]\n", parent: "early_custom_unique", child: "first_custom_unique"}, + {name: "ignored_partition_table", ignore: "[tables]\npatterns = [\"events_first\"]\n", parent: "early_custom_unique", child: "first_custom_unique"}, + {name: "ignored_leaf_without_fk", ignore: "[indexes]\npatterns = [\"late_custom_unique\"]\n", parent: "event_unique", child: "late_custom_unique", dropFK: true}, + {name: "ignored_child_of_invalid_parent", ignore: "[indexes]\npatterns = [\"late_partial_lookup\"]\n", parent: "partial_lookup", child: "late_partial_lookup"}, + {name: "ignored_unattached", ignore: "[indexes]\npatterns = [\"unattached_lookup\"]\n", absent: "unattached_lookup"}, + {name: "ignored_family", ignore: "[indexes]\npatterns = [\"event_unique\", \"*custom_unique\"]\n[constraints]\npatterns = [\"event_links_bucket_id_fkey\"]\n", absent: "event_unique"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Chdir(t.TempDir()) + if err := os.WriteFile(".pgschemaignore", []byte(tc.ignore), 0600); err != nil { + t.Fatal(err) + } + for _, multiFile := range []bool{false, true} { + for _, existingOutput := range []bool{false, true} { + t.Run(fmt.Sprintf("multi=%v/existing=%v", multiFile, existingOutput), func(t *testing.T) { + if _, err := conn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + setup); err != nil { + t.Fatal(err) + } + if tc.dropFK { + if _, err := conn.Exec(`ALTER TABLE event_links DROP CONSTRAINT event_links_bucket_id_fkey`); err != nil { + t.Fatal(err) + } + } + outputDir := t.TempDir() + path := filepath.Join(outputDir, "schema.sql") + const original = "-- existing output must survive a failed dump\n" + if existingOutput { + if err := os.WriteFile(path, []byte(original), 0600); err != nil { + t.Fatal(err) + } + } + output, err := ExecuteDump(&DumpConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", MultiFile: multiFile, File: path, + }) + if tc.child != "" { + want := "cannot dump index public." + tc.parent + ": attached index public." + tc.child + " is excluded" + if err == nil || !strings.Contains(err.Error(), want) || !strings.Contains(err.Error(), ".pgschemaignore") { + t.Fatalf("Expected actionable dependency error %q, got %v", want, err) + } + if output != "" { + t.Fatal("Failed dump returned partial SQL") + } + entries, readErr := os.ReadDir(outputDir) + if readErr != nil { + t.Fatal(readErr) + } + wantFiles := 0 + if existingOutput { + wantFiles = 1 + content, readErr := os.ReadFile(path) + if readErr != nil || string(content) != original { + t.Fatalf("Failed dump changed existing output: %q, %v", content, readErr) + } + } + if len(entries) != wantFiles { + t.Fatalf("Failed dump created files: %v", entries) + } + return + } + if err != nil { + t.Fatal(err) + } + if multiFile { + output, err = include.NewProcessor(outputDir).ProcessFile(path) + if err != nil { + t.Fatal(err) + } + } + if tc.absent != "" && strings.Contains(output, tc.absent) { + t.Fatalf("Dump contains ignored index %s", tc.absent) + } + if _, err := conn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + output); err != nil { + t.Fatalf("Allowed dump failed replay: %v", err) + } + }) + } + } + }) + } +} diff --git a/docs/cli/ignore.mdx b/docs/cli/ignore.mdx index dadc17ca..9d354ff6 100644 --- a/docs/cli/ignore.mdx +++ b/docs/cli/ignore.mdx @@ -119,6 +119,22 @@ patterns = ["deploy_bot"] # Ignore ALTER DEFAULT PRIVILEGES for deploy_bot The `[privileges]` section filters explicit grants (`GRANT ... TO role`), including column-level privileges. The `[default_privileges]` section filters `ALTER DEFAULT PRIVILEGES` statements. +## Partitioned indexes + +When `dump` retains a partitioned index, it must also retain that index's attached +children. If the inspector excludes an attached child because of an index or table +ignore pattern, `dump` returns an error naming the parent and child before writing +any output. Existing output files remain unchanged. `dump` performs the same check +for nested partitions. + +For example, `event_unique` has an attached child named `early_custom_unique`. +If `early_custom_unique` is ignored while `event_unique` is retained, `dump` returns +an error. +Remove the conflicting ignore pattern or add a negation pattern to retain the +child index and its table. Ignoring an unattached index remains supported. + +`plan` and `apply` retain their existing ignore behavior. + ## Constraints The `[constraints]` section matches table constraints by **constraint name** (primary keys, unique, foreign keys, check, and exclusion constraints). When a constraint is ignored, pgschema neither creates, drops, nor reports drift on it — it is left entirely to be managed out-of-band. diff --git a/internal/diff/diff.go b/internal/diff/diff.go index ed19f770..bd7108a3 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -511,8 +511,11 @@ func GenerateMigrationWithOptions(oldIR, newIR *ir.IR, targetSchema string, qual // GenerateDump creates an empty-to-target export while preserving partitioned // index identities. Migration entry points retain recursive index creation. -func GenerateDump(schema *ir.IR, targetSchema string, qualifySchema bool) []Diff { - return generateMigration(ir.NewIR(), schema, targetSchema, qualifySchema, 0, true) +func GenerateDump(schema *ir.IR, targetSchema string, qualifySchema bool) ([]Diff, error) { + if err := validatePartitionIndexDependencies(schema); err != nil { + return nil, err + } + return generateMigration(ir.NewIR(), schema, targetSchema, qualifySchema, 0, true), nil } func generateMigration(oldIR, newIR *ir.IR, targetSchema string, qualifySchema bool, targetMajorVersion int, dump bool) []Diff { diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go index ded87607..8d380b12 100644 --- a/internal/diff/partition_index.go +++ b/internal/diff/partition_index.go @@ -7,6 +7,38 @@ import ( "github.com/pgplex/pgschema/ir" ) +// validatePartitionIndexDependencies rejects filtered index families before any +// dump statements are generated. Migration generation still honors filtering. +func validatePartitionIndexDependencies(schema *ir.IR) error { + indexes := make(map[ir.IndexRef]*ir.Index) + var keys []ir.IndexRef + for _, namespace := range schema.Schemas { + for _, table := range namespace.Tables { + for _, index := range table.Indexes { + key := ir.IndexRef{Schema: index.Schema, Name: index.Name} + indexes[key] = index + keys = append(keys, key) + } + } + } + sort.Slice(keys, func(i, j int) bool { + if keys[i].Schema != keys[j].Schema { + return keys[i].Schema < keys[j].Schema + } + return keys[i].Name < keys[j].Name + }) + for _, key := range keys { + for _, child := range indexes[key].AttachedPartitions { + if _, exists := indexes[child]; !exists { + return fmt.Errorf("cannot dump index %s: attached index %s is excluded; adjust .pgschemaignore to include the child index and its table", + ir.QuoteIdentifier(key.Schema)+"."+ir.QuoteIdentifier(key.Name), + ir.QuoteIdentifier(child.Schema)+"."+ir.QuoteIdentifier(child.Name)) + } + } + } + return nil +} + // generatePartitionedIndexesSQL emits parent indexes after all partition tables // and leaf indexes exist. ON ONLY prevents PostgreSQL from creating or choosing // child indexes implicitly. Explicit attachments preserve names and validity. diff --git a/internal/diff/partition_index_test.go b/internal/diff/partition_index_test.go index 636408c1..fb4b1adc 100644 --- a/internal/diff/partition_index_test.go +++ b/internal/diff/partition_index_test.go @@ -13,7 +13,8 @@ func TestGenerateDumpPartitionIndexOrder(t *testing.T) { parent := &ir.Index{ Schema: "public", Table: "events", Name: "parent_lookup", Type: ir.IndexTypeRegular, Method: "btree", IsPartitioned: true, - Columns: []*ir.IndexColumn{{Name: "id", Position: 1}}, + Columns: []*ir.IndexColumn{{Name: "id", Position: 1}}, + AttachedPartitions: []ir.IndexRef{{Schema: "public", Name: "custom_lookup"}}, } child := &ir.Index{ Schema: "public", Table: "events_early", Name: "custom_lookup", @@ -45,7 +46,11 @@ func TestGenerateDumpPartitionIndexOrder(t *testing.T) { } var first string for run := 0; run < 5; run++ { - sql := buildSQLFromSteps(GenerateDump(schema, "public", false)) + steps, err := GenerateDump(schema, "public", false) + if err != nil { + t.Fatal(err) + } + sql := buildSQLFromSteps(steps) previous := -1 for _, stmt := range wantOrder { position := strings.Index(sql, stmt) @@ -68,3 +73,45 @@ func TestGenerateDumpPartitionIndexOrder(t *testing.T) { t.Fatalf("Expected existing recursive migration strategy:\n%s", migration) } } + +func TestGenerateDumpExcludedPartitionIndex(t *testing.T) { + for _, childSchema := range []string{"public", "other"} { + t.Run(childSchema, func(t *testing.T) { + parent := &ir.Index{ + Schema: "public", Table: "events", Name: "parent_lookup", + Type: ir.IndexTypeRegular, Method: "btree", IsPartitioned: true, + Columns: []*ir.IndexColumn{{Name: "id", Position: 1}}, + AttachedPartitions: []ir.IndexRef{{Schema: "public", Name: "child_lookup"}}, + } + child := &ir.Index{Schema: childSchema, Table: "events_early", Name: "child_lookup"} + schema := ir.NewIR() + schema.Schemas["public"] = &ir.Schema{Name: "public", Tables: map[string]*ir.Table{ + "events": { + Schema: "public", Name: "events", IsPartitioned: true, + PartitionStrategy: "RANGE", PartitionKey: "id", + Columns: []*ir.Column{{Name: "id", Position: 1, DataType: "integer"}}, + Indexes: map[string]*ir.Index{parent.Name: parent}, + }, + }} + if childSchema != "public" { + // A same-named index in another schema cannot satisfy the attachment. + schema.Schemas[childSchema] = &ir.Schema{Tables: map[string]*ir.Table{ + "events_early": {Indexes: map[string]*ir.Index{child.Name: child}}, + }} + } + steps, err := GenerateDump(schema, "public", false) + if err == nil || !strings.Contains(err.Error(), "index public.parent_lookup: attached index public.child_lookup is excluded") { + t.Fatalf("Expected excluded-child error, got %v", err) + } + if len(steps) != 0 { + t.Fatal("Failed dump returned partial SQL") + } + if childSchema == "public" { + migration := buildSQLFromSteps(GenerateMigration(ir.NewIR(), schema, "public")) + if !strings.Contains(migration, "CREATE INDEX IF NOT EXISTS parent_lookup ON events (id);") { + t.Fatalf("Dump validation changed migration generation:\n%s", migration) + } + } + }) + } +} diff --git a/ir/index_test.go b/ir/index_test.go new file mode 100644 index 00000000..30921fb3 --- /dev/null +++ b/ir/index_test.go @@ -0,0 +1,22 @@ +package ir + +import ( + "encoding/json" + "testing" +) + +func TestIndexAttachedPartitionsAreInspectionMetadata(t *testing.T) { + index := &Index{Schema: "public", Table: "events", Name: "event_lookup"} + before, err := json.Marshal(index) + if err != nil { + t.Fatal(err) + } + index.AttachedPartitions = []IndexRef{{Schema: "public", Name: "ignored_child"}} + after, err := json.Marshal(index) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Fatalf("Inspection metadata changed index serialization:\nbefore: %s\nafter: %s", before, after) + } +} diff --git a/ir/inspector.go b/ir/inspector.go index 78733c7f..27cf15bf 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -811,6 +811,15 @@ func (i *Inspector) buildIndexes(ctx context.Context, schema *IR, targetSchema s return err } + // Capture attachments before filtering so dump can detect missing children. + attachedPartitions := make(map[IndexRef][]IndexRef) + for _, row := range indexes { + if row.ParentIndex.Valid { + parent := IndexRef{Schema: row.ParentIndexSchema.String, Name: row.ParentIndex.String} + attachedPartitions[parent] = append(attachedPartitions[parent], IndexRef{Schema: row.Schemaname, Name: row.Indexname}) + } + } + for _, indexRow := range indexes { schemaName := indexRow.Schemaname tableName := indexRow.Tablename @@ -845,20 +854,21 @@ func (i *Inspector) buildIndexes(ctx context.Context, schema *IR, targetSchema s } index := &Index{ - Schema: schemaName, - Table: tableName, - Name: indexName, - Type: indexType, - Method: method, - IncludeColumns: indexRow.IncludeColumns, - IsPartial: isPartial, - IsExpression: hasExpressions, - Where: "", - StorageParameters: indexRow.Reloptions, - ParentIndex: indexRow.ParentIndex.String, - ParentIndexSchema: indexRow.ParentIndexSchema.String, - Comment: comment, - Columns: []*IndexColumn{}, + Schema: schemaName, + Table: tableName, + Name: indexName, + Type: indexType, + Method: method, + IncludeColumns: indexRow.IncludeColumns, + IsPartial: isPartial, + IsExpression: hasExpressions, + Where: "", + StorageParameters: indexRow.Reloptions, + ParentIndex: indexRow.ParentIndex.String, + ParentIndexSchema: indexRow.ParentIndexSchema.String, + AttachedPartitions: attachedPartitions[IndexRef{Schema: schemaName, Name: indexName}], + Comment: comment, + Columns: []*IndexColumn{}, } // Check for NULLS NOT DISTINCT (PostgreSQL 15+) diff --git a/ir/ir.go b/ir/ir.go index 7b71f6a5..fbbba5ee 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -282,6 +282,12 @@ const ( ConstraintTypeExclusion ConstraintType = "EXCLUSION" ) +// IndexRef identifies an index within a schema. +type IndexRef struct { + Schema string + Name string +} + // Index represents a database index type Index struct { Schema string `json:"schema"` @@ -300,6 +306,10 @@ type Index struct { ParentIndexSchema string `json:"parent_index_schema,omitempty"` StorageParameters []string `json:"storage_parameters,omitempty"` // WITH (fillfactor=100, ...) Comment string `json:"comment,omitempty"` + // AttachedPartitions records catalog attachments, including ignored children. + // Dump validation uses it to detect excluded dependencies. It is inspection + // metadata, not part of the managed schema or its fingerprint. + AttachedPartitions []IndexRef `json:"-"` } // IndexColumn represents a column within an index From 676c68fa7e98e4e0b96a60674162e5addade0840 Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Tue, 13 Oct 2026 00:02:59 +0200 Subject: [PATCH 4/6] fix(plan): preserve index identities in new partition hierarchies Reuse explicit index creation and attachment ordering for complete new partition hierarchies. Preserve recursive migration behavior for existing or filtered hierarchies and defer dependent foreign keys until unique indexes are ready. Cover native apply, direct SQL replay, incremental migrations, filtering, and transactional attachment failure with independent synthetic fixtures. --- cmd/partition_index_migration_test.go | 314 ++++++++++++++++++++++++++ cmd/plan/plan.go | 11 + cmd/plan/plan_test.go | 15 +- docs/cli/plan.mdx | 17 ++ internal/diff/collector.go | 3 - internal/diff/diff.go | 15 +- internal/diff/index.go | 5 - internal/diff/partition_index.go | 182 +++++++++++---- internal/diff/partition_index_test.go | 89 +++++++- internal/diff/qualify_schema_test.go | 4 +- internal/diff/table.go | 13 +- ir/index_test.go | 10 +- ir/inspector.go | 7 + ir/ir.go | 3 + 14 files changed, 601 insertions(+), 87 deletions(-) create mode 100644 cmd/partition_index_migration_test.go diff --git a/cmd/partition_index_migration_test.go b/cmd/partition_index_migration_test.go new file mode 100644 index 00000000..59915f01 --- /dev/null +++ b/cmd/partition_index_migration_test.go @@ -0,0 +1,314 @@ +package cmd + +import ( + "database/sql" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/pgplex/pgschema/cmd/apply" + planCmd "github.com/pgplex/pgschema/cmd/plan" + "github.com/pgplex/pgschema/testutil" +) + +// TestPlanAndApplyPartitionIndexIdentity compares native migration and direct SQL +// replay with an independently created source, rather than a previous migration. +func TestPlanAndApplyPartitionIndexIdentity(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + t.Chdir(t.TempDir()) + source := testutil.SetupPostgres(t) + defer source.Stop() + sourceConn, _, _, _, _, _ := testutil.ConnectToPostgres(t, source) + defer sourceConn.Close() + target := testutil.SetupPostgres(t) + defer target.Stop() + targetConn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, target) + defer targetConn.Close() + + const desired = ` +CREATE TABLE events (bucket int, id int, label text) PARTITION BY RANGE (bucket); +CREATE TABLE events_early PARTITION OF events FOR VALUES FROM (0) TO (10); +CREATE TABLE events_late PARTITION OF events FOR VALUES FROM (10) TO (20); +CREATE INDEX event_lookup ON ONLY events (label); +CREATE INDEX early_custom_lookup ON events_early (label); +CREATE INDEX late_custom_lookup ON events_late (label); +ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup; +ALTER INDEX event_lookup ATTACH PARTITION late_custom_lookup; +CREATE INDEX early_unattached_lookup ON events_early (label); +CREATE UNIQUE INDEX event_unique ON ONLY events (bucket, id); +CREATE UNIQUE INDEX early_custom_unique ON events_early (bucket, id); +CREATE UNIQUE INDEX late_custom_unique ON events_late (bucket, id); +ALTER INDEX event_unique ATTACH PARTITION early_custom_unique; +ALTER INDEX event_unique ATTACH PARTITION late_custom_unique; +CREATE TABLE event_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCES events (bucket, id)); +CREATE INDEX event_partial ON ONLY events ((lower(label))) WHERE id > 0; +CREATE INDEX early_custom_partial ON events_early ((lower(label))) WHERE id > 0; +CREATE INDEX late_unattached_partial ON events_late ((lower(label))) WHERE id > 0; +ALTER INDEX event_partial ATTACH PARTITION early_custom_partial; +CREATE TABLE archive_events (bucket int, label text) PARTITION BY RANGE (bucket); +CREATE TABLE archive_events_early PARTITION OF archive_events FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); +CREATE TABLE archive_events_first PARTITION OF archive_events_early FOR VALUES FROM (0) TO (5); +CREATE TABLE archive_events_second PARTITION OF archive_events_early FOR VALUES FROM (5) TO (10); +CREATE INDEX a_archive_lookup ON ONLY archive_events (label); +CREATE INDEX z_archive_lookup ON ONLY archive_events_early (label); +CREATE INDEX first_custom_lookup ON archive_events_first (label); +CREATE INDEX second_custom_lookup ON archive_events_second (label); +ALTER INDEX z_archive_lookup ATTACH PARTITION first_custom_lookup; +ALTER INDEX z_archive_lookup ATTACH PARTITION second_custom_lookup; +ALTER INDEX a_archive_lookup ATTACH PARTITION z_archive_lookup; +CREATE INDEX first_unattached_lookup ON archive_events_first (label); +CREATE INDEX "Event.Lookup" ON ONLY events (id DESC); +CREATE INDEX "Early.Lookup" ON events_early (id DESC); +CREATE INDEX "Late.Lookup" ON events_late (id DESC); +ALTER INDEX "Event.Lookup" ATTACH PARTITION "Early.Lookup"; +ALTER INDEX "Event.Lookup" ATTACH PARTITION "Late.Lookup"; +CREATE TABLE empty_events (id int) PARTITION BY RANGE (id); +CREATE INDEX empty_lookup ON empty_events (id); +COMMENT ON INDEX event_lookup IS 'Parent lookup'; +COMMENT ON INDEX early_custom_lookup IS 'Named child lookup'; +` + for _, initial := range []struct{ name, sql, desiredPrefix string }{ + {name: "empty"}, + {name: "unrelated_table", sql: "CREATE TABLE unrelated (id integer);\n", desiredPrefix: "CREATE TABLE unrelated (id integer);\n"}, + {name: "existing_referencing_table", sql: "CREATE TABLE event_links (bucket int, id int);\n"}, + } { + for _, executor := range []string{"native", "sql"} { + t.Run(initial.name+"/"+executor, func(t *testing.T) { + if _, err := sourceConn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + initial.desiredPrefix + desired); err != nil { + t.Fatal(err) + } + wantCount, wantCatalog := migrationIndexCatalog(t, sourceConn) + if wantCount != 19 { + t.Fatalf("Expected 19 source indexes, got %d", wantCount) + } + if _, err := targetConn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + initial.sql); err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "schema.sql") + if err := os.WriteFile(path, []byte(initial.desiredPrefix+desired), 0600); err != nil { + t.Fatal(err) + } + config := &planCmd.PlanConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + } + migration, err := planCmd.GeneratePlan(config, sharedEmbeddedPG) + if err != nil { + t.Fatal(err) + } + var statements []string + for _, group := range migration.Groups { + for _, step := range group.Steps { + if step.SQL != "" { + statements = append(statements, step.SQL) + } + } + } + generatedSQL := strings.Join(statements, "\n") + if !strings.Contains(generatedSQL, "event_lookup ON ONLY events") || + !strings.Contains(generatedSQL, "ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup") { + t.Errorf("Generated migration does not preserve explicit index attachments:\n%s", generatedSQL) + } + if strings.Contains(generatedSQL, "CONCURRENTLY") { + t.Errorf("New-table index creation must remain synchronous:\n%s", generatedSQL) + } + if initial.name == "empty" && executor == "native" { + if len(migration.Groups) != 1 { + t.Fatalf("Expected one transactional bootstrap group, got %d", len(migration.Groups)) + } + // A failed attachment must propagate without a recursive fallback. + // PostgreSQL must roll back the ordinary group before a retry. + failedAttachment := false + for i := range migration.Groups[0].Steps { + step := &migration.Groups[0].Steps[i] + if step.SQL != "ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup;" { + continue + } + originalSQL := step.SQL + step.SQL = "ALTER INDEX event_lookup ATTACH PARTITION missing_lookup;" + applyErr := apply.ApplyMigration(&apply.ApplyConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", Plan: migration, + AutoApprove: true, NoColor: true, Quiet: true, + }, nil) + step.SQL = originalSQL + if applyErr == nil || !strings.Contains(applyErr.Error(), "missing_lookup") { + t.Fatalf("Expected attachment failure, got %v", applyErr) + } + var tableCount int + if err := targetConn.QueryRow(`SELECT count(*) FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = 'public' AND c.relkind IN ('r', 'p')`).Scan(&tableCount); err != nil { + t.Fatal(err) + } + if count, _ := migrationIndexCatalog(t, targetConn); count != 0 || tableCount != 0 { + t.Fatalf("Failed group left %d tables and %d indexes", tableCount, count) + } + failedAttachment = true + break + } + if !failedAttachment { + t.Fatal("No explicit attachment available for failure test") + } + } + if executor == "native" { + err = apply.ApplyMigration(&apply.ApplyConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", Plan: migration, + AutoApprove: true, NoColor: true, Quiet: true, + }, nil) + } else { + // Use the emitted SQL directly to isolate generation from the executor. + for _, statement := range statements { + if _, err = targetConn.Exec(statement); err != nil { + break + } + } + } + if err != nil { + t.Fatalf("Migration failed: %v", err) + } + gotCount, gotCatalog := migrationIndexCatalog(t, targetConn) + if gotCatalog != wantCatalog { + t.Errorf("Index catalog mismatch: expected %d indexes, got %d\nwant: %s\ngot: %s", wantCount, gotCount, wantCatalog, gotCatalog) + } + if got, want := migrationFKCatalog(t, targetConn), migrationFKCatalog(t, sourceConn); got != want { + t.Errorf("FK catalog mismatch:\nwant: %s\ngot: %s", want, got) + } + followUp, err := planCmd.GeneratePlan(config, sharedEmbeddedPG) + if err != nil { + t.Fatal(err) + } + if len(followUp.Groups) != 0 { + t.Errorf("Expected empty follow-up plan, got %+v", followUp.Groups) + } + + // Keep incremental creation under an existing parent and unrelated + // table alterations covered after the independent bootstrap check. + const incremental = `ALTER TABLE event_links ADD COLUMN note text; +CREATE TABLE events_future PARTITION OF events FOR VALUES FROM (20) TO (30);` + if _, err := sourceConn.Exec(incremental); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(initial.desiredPrefix+desired+incremental), 0600); err != nil { + t.Fatal(err) + } + if err := apply.ApplyMigration(&apply.ApplyConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + AutoApprove: true, NoColor: true, Quiet: true, + }, sharedEmbeddedPG); err != nil { + t.Fatalf("Incremental migration failed: %v", err) + } + _, wantCatalog = migrationIndexCatalog(t, sourceConn) + _, gotCatalog = migrationIndexCatalog(t, targetConn) + if gotCatalog != wantCatalog { + t.Errorf("Incremental migration changed index identity:\nwant: %s\ngot: %s", wantCatalog, gotCatalog) + } + if got, want := migrationFKCatalog(t, targetConn), migrationFKCatalog(t, sourceConn); got != want { + t.Errorf("Incremental migration changed FKs:\nwant: %s\ngot: %s", want, got) + } + followUp, err = planCmd.GeneratePlan(config, sharedEmbeddedPG) + if err != nil || len(followUp.Groups) != 0 { + t.Fatalf("Incremental follow-up plan is not empty: %+v, %v", followUp, err) + } + }) + } + } +} + +func migrationIndexCatalog(t *testing.T, conn *sql.DB) (int, string) { + t.Helper() + var count int + var catalog string + err := conn.QueryRow(` +SELECT count(*), COALESCE(jsonb_agg(to_jsonb(x) ORDER BY name), '[]'::jsonb)::text +FROM ( + SELECT c.relname AS name, t.relname AS table_name, + pg_get_indexdef(c.oid) AS definition, i.indisvalid AS valid, + i.indisready AS ready, i.indislive AS live, + obj_description(c.oid, 'pg_class') AS comment, + ARRAY(SELECT pn.nspname || '.' || p.relname + FROM pg_inherits h JOIN pg_class p ON p.oid = h.inhparent + JOIN pg_namespace pn ON pn.oid = p.relnamespace + WHERE h.inhrelid = c.oid ORDER BY h.inhseqno) AS parents + FROM pg_index i JOIN pg_class c ON c.oid = i.indexrelid + JOIN pg_class t ON t.oid = i.indrelid JOIN pg_namespace n ON n.oid = t.relnamespace + WHERE n.nspname = 'public' +) x`).Scan(&count, &catalog) + if err != nil { + t.Fatal(err) + } + return count, catalog +} + +// Compare declared FKs. PostgreSQL creates internal child constraints for a +// partitioned target; pgschema does not manage those constraints independently. +func migrationFKCatalog(t *testing.T, conn *sql.DB) string { + t.Helper() + var catalog string + err := conn.QueryRow(` +SELECT COALESCE(jsonb_agg(to_jsonb(x) ORDER BY table_name, name), '[]'::jsonb)::text +FROM ( + SELECT c.conrelid::regclass::text AS table_name, c.conname AS name, + pg_get_constraintdef(c.oid) AS definition + FROM pg_constraint c JOIN pg_namespace n ON n.oid = c.connamespace + WHERE n.nspname = 'public' AND c.contype = 'f' AND c.conparentid = 0 +) x`).Scan(&catalog) + if err != nil { + t.Fatal(err) + } + return catalog +} + +func TestPlanPartitionIndexFilteringCompatibility(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + target := testutil.SetupPostgres(t) + defer target.Stop() + conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, target) + defer conn.Close() + const desired = ` +CREATE TABLE events (id int, label text) PARTITION BY RANGE (id); +CREATE TABLE events_early PARTITION OF events FOR VALUES FROM (0) TO (10); +CREATE TABLE events_late PARTITION OF events FOR VALUES FROM (10) TO (20); +CREATE INDEX parent_lookup ON ONLY events (label); +CREATE INDEX custom_lookup ON events_early (label); +ALTER INDEX parent_lookup ATTACH PARTITION custom_lookup; +` + for _, tc := range []struct{ name, ignore string }{ + {name: "ignored_attached_index", ignore: "[indexes]\npatterns = [\"custom_lookup\"]\n"}, + {name: "ignored_attached_table", ignore: "[tables]\npatterns = [\"events_early\"]\n"}, + {name: "ignored_table_without_index", ignore: "[tables]\npatterns = [\"events_late\"]\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Chdir(t.TempDir()) + if err := os.WriteFile(".pgschemaignore", []byte(tc.ignore), 0600); err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "schema.sql") + if err := os.WriteFile(path, []byte(desired), 0600); err != nil { + t.Fatal(err) + } + migration, err := planCmd.GeneratePlan(&planCmd.PlanConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + }, sharedEmbeddedPG) + if err != nil { + t.Fatal(err) + } + var sqlText strings.Builder + for _, group := range migration.Groups { + for _, step := range group.Steps { + sqlText.WriteString(step.SQL) + } + } + text := sqlText.String() + if !strings.Contains(text, "CREATE INDEX IF NOT EXISTS parent_lookup ON events (label);") || strings.Contains(text, "ATTACH PARTITION") { + t.Fatalf("Filtered hierarchy must retain recursive migration behavior:\n%s", text) + } + }) + } +} diff --git a/cmd/plan/plan.go b/cmd/plan/plan.go index 9125fcbf..c58a0354 100644 --- a/cmd/plan/plan.go +++ b/cmd/plan/plan.go @@ -579,6 +579,12 @@ func normalizeSchemaNames(irData *ir.IR, fromSchema, toSchema string) { } } + for j := range table.AllPartitionChildren { + if table.AllPartitionChildren[j].Schema == fromSchema { + table.AllPartitionChildren[j].Schema = toSchema + } + } + // Normalize schema names in indexes for _, index := range table.Indexes { if index.Schema == fromSchema { @@ -587,6 +593,11 @@ func normalizeSchemaNames(irData *ir.IR, fromSchema, toSchema string) { if index.ParentIndexSchema == fromSchema { index.ParentIndexSchema = toSchema } + for j := range index.AttachedPartitions { + if index.AttachedPartitions[j].Schema == fromSchema { + index.AttachedPartitions[j].Schema = toSchema + } + } index.Where = stripQualifiers(replaceString(index.Where)) if index.IsExpression { for _, col := range index.Columns { diff --git a/cmd/plan/plan_test.go b/cmd/plan/plan_test.go index e9aa4613..4cec9bad 100644 --- a/cmd/plan/plan_test.go +++ b/cmd/plan/plan_test.go @@ -292,11 +292,15 @@ func TestNormalizeSchemaNames_PreservesViewTableAliasesMatchingTargetSchema(t *t func TestNormalizeSchemaNames_IndexAttachment(t *testing.T) { const temporary = "pgschema_tmp_index_attachment" - local := &ir.Index{Schema: temporary, ParentIndex: "parent", ParentIndexSchema: temporary} + local := &ir.Index{Schema: temporary, ParentIndex: "parent", ParentIndexSchema: temporary, + AttachedPartitions: []ir.IndexRef{{Schema: temporary, Name: "leaf"}, {Schema: "other", Name: "outside_leaf"}}, + } external := &ir.Index{Schema: temporary, ParentIndex: "outside_parent", ParentIndexSchema: "other"} data := &ir.IR{Schemas: map[string]*ir.Schema{ temporary: {Name: temporary, Tables: map[string]*ir.Table{ - "child": {Schema: temporary, Name: "child", Indexes: map[string]*ir.Index{"local": local, "external": external}}, + "child": {Schema: temporary, Name: "child", Indexes: map[string]*ir.Index{"local": local, "external": external}, + AllPartitionChildren: []ir.TableDependency{{Schema: temporary, Name: "leaf"}, {Schema: "other", Name: "outside_leaf"}}, + }, }}, }} normalizeSchemaNames(data, temporary, "public") @@ -306,4 +310,11 @@ func TestNormalizeSchemaNames_IndexAttachment(t *testing.T) { if external.ParentIndexSchema != "other" || external.ParentIndex != "outside_parent" { t.Fatalf("Cross-schema index attachment changed: %+v", external) } + if local.AttachedPartitions[0].Schema != "public" || local.AttachedPartitions[1].Schema != "other" { + t.Fatalf("Incorrect attached-index metadata normalization: %+v", local.AttachedPartitions) + } + children := data.Schemas["public"].Tables["child"].AllPartitionChildren + if children[0].Schema != "public" || children[1].Schema != "other" { + t.Fatalf("Incorrect partition-table metadata normalization: %+v", children) + } } diff --git a/docs/cli/plan.mdx b/docs/cli/plan.mdx index f6bb322b..5ba553f9 100644 --- a/docs/cli/plan.mdx +++ b/docs/cli/plan.mdx @@ -16,6 +16,23 @@ The plan command follows infrastructure-as-code principles similar to Terraform: By default, pgschema uses an embedded PostgreSQL instance to validate your desired state SQL. Cross-schema foreign keys to unmanaged tables (for example Supabase `auth.users`) work with the default embedded instance when you [ignore those schemas](/cli/ignore). For PostgreSQL extensions, or when referenced tables are not on the target, see [External Plan Database](/cli/plan-db). +## Creating partitioned indexes + +When `plan` creates an entirely new managed partition hierarchy and all recorded +table and index dependencies are present, it preserves standalone index names and +attachments. It creates the tables and leaf indexes first, then parent indexes +with `ON ONLY`, explicit attachments, and dependent foreign keys. + +For example, creating new `events` tables preserves the attachment of a named +child index `early_custom_lookup` to `event_lookup`. An equivalent unattached index +stays unattached. The same creation order applies when unrelated tables already +exist in the target schema. + +For hierarchies containing existing tables, unmanaged ancestors, or excluded +dependencies, migration generation retains recursive index creation. This does +not reconcile index attachments on existing tables. Constraint-backed indexes +remain under constraint management. + ## Basic Usage ```bash diff --git a/internal/diff/collector.go b/internal/diff/collector.go index 7bf165b1..68ceb90a 100644 --- a/internal/diff/collector.go +++ b/internal/diff/collector.go @@ -18,9 +18,6 @@ type diffCollector struct { // for the target schema (set by `dump --qualify-schema`). Default false preserves // the standard "smart qualification" (omit the target-schema prefix). qualifySchema bool - - // dump preserves explicit partitioned index creation and attachment order. - dump bool } // newDiffCollector creates a new diffCollector diff --git a/internal/diff/diff.go b/internal/diff/diff.go index bd7108a3..1d198a21 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -278,6 +278,7 @@ type ddlDiff struct { droppedTables []*ir.Table modifiedTables []*tableDiff allNewTables map[string]*ir.Table + partitionIndexes *partitionIndexCreationPlan addedViews []*ir.View droppedViews []*ir.View modifiedViews []*viewDiff @@ -510,7 +511,7 @@ func GenerateMigrationWithOptions(oldIR, newIR *ir.IR, targetSchema string, qual } // GenerateDump creates an empty-to-target export while preserving partitioned -// index identities. Migration entry points retain recursive index creation. +// index identities. Migrations use the same schedule for complete new hierarchies. func GenerateDump(schema *ir.IR, targetSchema string, qualifySchema bool) ([]Diff, error) { if err := validatePartitionIndexDependencies(schema); err != nil { return nil, err @@ -1643,7 +1644,7 @@ func generateMigration(oldIR, newIR *ir.IR, targetSchema string, qualifySchema b // Create a diffCollector and generate SQL collector := newDiffCollector() collector.qualifySchema = qualifySchema - collector.dump = dump + diff.partitionIndexes = planPartitionIndexCreation(diff.addedTables, newTables, dump) diff.collectMigrationSQL(targetSchema, collector) return collector.diffs } @@ -2024,7 +2025,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, d.allNewTables) + deferredPolicies1, deferredConstraints1 := generateCreateTablesSQL(tablesWithoutDeps, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs, d.allNewTables, d.partitionIndexes) // Build view lookup - needed for detecting functions that depend on views newViewLookup := buildViewLookup(d.addedViews) @@ -2123,7 +2124,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, d.allNewTables) + deferredPolicies2, deferredConstraints2 := generateCreateTablesSQL(tablesWithDeps, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs, d.allNewTables, d.partitionIndexes) // Create functions WITHOUT view deps that reference new tables (now that all tables exist) // These functions query tables that were created in tablesWithDeps above (issue #530) @@ -2135,7 +2136,7 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateCreateTypesSQL(domainsWithTableDeps, targetSchema, collector) // Create tables that use table-dep domains as column types (now that those domains exist). - deferredPolicies3, deferredConstraints3 := generateCreateTablesSQL(tablesAfterTableDomains, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs, d.allNewTables) + deferredPolicies3, deferredConstraints3 := generateCreateTablesSQL(tablesAfterTableDomains, targetSchema, collector, existingTables, shouldDeferPolicy, d.suppressedInlineFKs, d.allNewTables, d.partitionIndexes) // Create functions that reference tables in the last table batch above. generateCreateFunctionsSQL(functionsAfterAllTables, targetSchema, collector) @@ -2146,9 +2147,7 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateSequenceComment(seq, targetSchema, DiffOperationCreate, collector) } - if collector.dump { - generatePartitionedIndexesSQL(d.addedTables, targetSchema, collector) - } + d.partitionIndexes.generateSQL(targetSchema, collector) // Add deferred foreign key constraints from ALL batches AFTER all tables are created // This ensures FK references to tables in any batch work correctly diff --git a/internal/diff/index.go b/internal/diff/index.go index d450eeb6..0ea5af32 100644 --- a/internal/diff/index.go +++ b/internal/diff/index.go @@ -28,11 +28,6 @@ func generateCreateIndexesSQLWithType(indexes []*ir.Index, targetSchema string, continue } - if collector.dump && index.IsPartitioned { - // Export these after every table exists. Creating a partition after - // its parent's index would make PostgreSQL generate child indexes. - continue - } generateCreateIndexSQL(index, targetSchema, collector, indexType, commentType, false) } } diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go index 8d380b12..8cd3b1a8 100644 --- a/internal/diff/partition_index.go +++ b/internal/diff/partition_index.go @@ -39,57 +39,161 @@ func validatePartitionIndexDependencies(schema *ir.IR) error { return nil } -// generatePartitionedIndexesSQL emits parent indexes after all partition tables -// and leaf indexes exist. ON ONLY prevents PostgreSQL from creating or choosing -// child indexes implicitly. Explicit attachments preserve names and validity. -func generatePartitionedIndexesSQL(tables []*ir.Table, targetSchema string, collector *diffCollector) { - type indexKey struct{ schema, name string } - parents := make(map[indexKey]*ir.Index) - children := make(map[indexKey][]*ir.Index) - var keys []indexKey - for _, table := range tables { +// partitionIndexCreationPlan belongs to ddlDiff. It schedules explicit index +// creation only after the tables and named leaf indexes are available. +// Migrations select complete, entirely new table hierarchies. Existing or +// filtered hierarchies retain recursive creation for compatibility. +// Dump selects every managed partitioned index after its strict validation. +type partitionIndexCreationPlan struct { + indexes map[ir.IndexRef]*ir.Index + children map[ir.IndexRef][]*ir.Index + uniqueTables map[string]bool +} + +func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[string]*ir.Table, dump bool) *partitionIndexCreationPlan { + plan := &partitionIndexCreationPlan{ + indexes: make(map[ir.IndexRef]*ir.Index), + children: make(map[ir.IndexRef][]*ir.Index), + uniqueTables: make(map[string]bool), + } + added := make(map[string]*ir.Table) + indexes := make(map[ir.IndexRef]*ir.Index) + for _, table := range addedTables { + added[table.Schema+"."+table.Name] = table for _, index := range table.Indexes { - if index.Type == ir.IndexTypePrimary { - continue // Primary indexes are created by constraints. + if index.Type != ir.IndexTypePrimary { + indexes[ir.IndexRef{Schema: index.Schema, Name: index.Name}] = index } - key := indexKey{index.Schema, index.Name} - if index.IsPartitioned { - parents[key] = index - keys = append(keys, key) + } + } + + roots := make(map[string]string) + blocked := make(map[string]bool) + if !dump { + for key, table := range desiredTables { + root := partitionHierarchyRoot(table, desiredTables) + roots[key] = root + if root == "" { + continue + } + if added[key] == nil || table.IsExternal { + blocked[root] = true } - if index.ParentIndex != "" { - parent := indexKey{index.ParentIndexSchema, index.ParentIndex} - children[parent] = append(children[parent], index) + // A missing table can change index validity even when it has no + // attached index. Check table membership as well as index edges. + for _, child := range table.AllPartitionChildren { + if added[child.Schema+"."+child.Name] == nil { + blocked[root] = true + } + } + for _, index := range table.Indexes { + for _, child := range index.AttachedPartitions { + if indexes[child] == nil { + blocked[root] = true + } + } + if index.ParentIndex != "" && indexes[ir.IndexRef{Schema: index.ParentIndexSchema, Name: index.ParentIndex}] == nil { + blocked[root] = true + } } } } + + for _, table := range addedTables { + key := table.Schema + "." + table.Name + if !dump && (roots[key] == "" || blocked[roots[key]]) { + continue + } + for _, index := range table.Indexes { + if index.IsPartitioned && index.Type != ir.IndexTypePrimary { + plan.indexes[ir.IndexRef{Schema: index.Schema, Name: index.Name}] = index + if index.Type == ir.IndexTypeUnique { + plan.uniqueTables[key] = true + } + } + } + } + for _, index := range indexes { + parent := ir.IndexRef{Schema: index.ParentIndexSchema, Name: index.ParentIndex} + if plan.indexes[parent] != nil { + plan.children[parent] = append(plan.children[parent], index) + } + } + return plan +} + +// partitionHierarchyRoot returns an empty key for incomplete or cyclic ancestry. +func partitionHierarchyRoot(table *ir.Table, tables map[string]*ir.Table) string { + seen := make(map[string]bool) + for table != nil && !table.IsExternal { + key := table.Schema + "." + table.Name + if seen[key] { + return "" + } + seen[key] = true + if table.PartitionOf == "" { + return key + } + parentSchema := table.PartitionOfSchema + if parentSchema == "" { + parentSchema = table.Schema + } + table = tables[parentSchema+"."+table.PartitionOf] + } + return "" +} + +func (p *partitionIndexCreationPlan) contains(index *ir.Index) bool { + return p != nil && p.indexes[ir.IndexRef{Schema: index.Schema, Name: index.Name}] != nil +} + +func (p *partitionIndexCreationPlan) defersForeignKey(table *ir.Table, constraint *ir.Constraint) bool { + if p == nil || constraint == nil || constraint.Type != ir.ConstraintTypeForeignKey { + return false + } + schema := constraint.ReferencedSchema + if schema == "" { + schema = table.Schema + } + return p.uniqueTables[schema+"."+constraint.ReferencedTable] +} + +// generateSQL creates partitioned children before their parents. ON ONLY avoids +// implicitly creating or choosing leaf indexes; only recorded children attach. +func (p *partitionIndexCreationPlan) generateSQL(targetSchema string, collector *diffCollector) { + if p == nil { + return + } + keys := make([]ir.IndexRef, 0, len(p.indexes)) + for key := range p.indexes { + keys = append(keys, key) + } sort.Slice(keys, func(i, j int) bool { - if keys[i].schema != keys[j].schema { - return keys[i].schema < keys[j].schema + if keys[i].Schema != keys[j].Schema { + return keys[i].Schema < keys[j].Schema } - return keys[i].name < keys[j].name + return keys[i].Name < keys[j].Name }) - created := make(map[indexKey]bool) - var create func(indexKey) - create = func(key indexKey) { + created := make(map[ir.IndexRef]bool) + var create func(ir.IndexRef) + create = func(key ir.IndexRef) { if created[key] { return } created[key] = true - attached := children[key] + attached := p.children[key] sort.Slice(attached, func(i, j int) bool { if attached[i].Schema != attached[j].Schema { return attached[i].Schema < attached[j].Schema } return attached[i].Name < attached[j].Name }) - // A partitioned child must exist before its own attachment is emitted. for _, child := range attached { if child.IsPartitioned { - create(indexKey{child.Schema, child.Name}) + create(ir.IndexRef{Schema: child.Schema, Name: child.Name}) } } - index := parents[key] + index := p.indexes[key] generateCreateIndexSQL(index, targetSchema, collector, DiffTypeTableIndex, DiffTypeTableIndexComment, true) for _, child := range attached { sql := fmt.Sprintf("ALTER INDEX %s ATTACH PARTITION %s;", @@ -106,25 +210,3 @@ func generatePartitionedIndexesSQL(tables []*ir.Table, targetSchema string, coll create(key) } } - -// referencesPartitionedUniqueIndex identifies FKs that may need a postponed -// unique index. Those FKs must wait until the index and its attachments exist. -func referencesPartitionedUniqueIndex(table *ir.Table, constraint *ir.Constraint, tables map[string]*ir.Table) bool { - if constraint == nil || constraint.Type != ir.ConstraintTypeForeignKey { - return false - } - schema := constraint.ReferencedSchema - if schema == "" { - schema = table.Schema - } - parent := tables[schema+"."+constraint.ReferencedTable] - if parent == nil || !parent.IsPartitioned { - return false - } - for _, index := range parent.Indexes { - if index.Type == ir.IndexTypeUnique { - return true - } - } - return false -} diff --git a/internal/diff/partition_index_test.go b/internal/diff/partition_index_test.go index fb4b1adc..72d3d85e 100644 --- a/internal/diff/partition_index_test.go +++ b/internal/diff/partition_index_test.go @@ -7,9 +7,7 @@ import ( "github.com/pgplex/pgschema/ir" ) -// TestGenerateDumpPartitionIndexOrder keeps the export-only index strategy out -// of migration generation and checks ordering independently of the formatter. -func TestGenerateDumpPartitionIndexOrder(t *testing.T) { +func partitionIndexTestSchema() *ir.IR { parent := &ir.Index{ Schema: "public", Table: "events", Name: "parent_lookup", Type: ir.IndexTypeRegular, Method: "btree", IsPartitioned: true, @@ -37,6 +35,13 @@ func TestGenerateDumpPartitionIndexOrder(t *testing.T) { Indexes: map[string]*ir.Index{child.Name: child}, }, }} + return schema +} + +// TestGenerateDumpPartitionIndexOrder checks the shared creation order for a +// complete new hierarchy independently of the dump formatter. +func TestGenerateDumpPartitionIndexOrder(t *testing.T) { + schema := partitionIndexTestSchema() wantOrder := []string{ "CREATE TABLE IF NOT EXISTS events (", "CREATE TABLE IF NOT EXISTS events_early PARTITION OF events", @@ -66,11 +71,8 @@ func TestGenerateDumpPartitionIndexOrder(t *testing.T) { } } migration := buildSQLFromSteps(GenerateMigration(ir.NewIR(), schema, "public")) - if strings.Contains(migration, "ON ONLY") || strings.Contains(migration, "ATTACH PARTITION") { - t.Fatalf("Dump-only index strategy changed migration generation:\n%s", migration) - } - if !strings.Contains(migration, "CREATE INDEX IF NOT EXISTS parent_lookup ON events (id);") { - t.Fatalf("Expected existing recursive migration strategy:\n%s", migration) + if migration != first { + t.Fatalf("New hierarchy migration differs from dump:\n%s", migration) } } @@ -115,3 +117,74 @@ func TestGenerateDumpExcludedPartitionIndex(t *testing.T) { }) } } + +func TestPartitionIndexCreationEligibility(t *testing.T) { + for _, tc := range []struct { + name string + want bool + }{ + {name: "complete", want: true}, + {name: "unrelated_existing_hierarchy", want: true}, + {name: "intentionally_unattached", want: true}, + {name: "ignored_child"}, + {name: "ignored_parent"}, + {name: "ignored_table_without_attachments"}, + {name: "external_child"}, + {name: "existing_root"}, + {name: "existing_child"}, + {name: "unmanaged_ancestor"}, + } { + t.Run(tc.name, func(t *testing.T) { + schema := partitionIndexTestSchema() + root := schema.Schemas["public"].Tables["events"] + child := schema.Schemas["public"].Tables["events_early"] + parentIndex := root.Indexes["parent_lookup"] + childIndex := child.Indexes["custom_lookup"] + parentIndex.Type = ir.IndexTypeUnique + root.AllPartitionChildren = []ir.TableDependency{{Schema: "public", Name: "events_early"}} + desired := map[string]*ir.Table{"public.events": root, "public.events_early": child} + added := []*ir.Table{root, child} + switch tc.name { + case "unrelated_existing_hierarchy": + desired["public.old_events"] = &ir.Table{Schema: "public", Name: "old_events", IsPartitioned: true} + case "intentionally_unattached": + parentIndex.AttachedPartitions = nil + childIndex.ParentIndex = "" + childIndex.ParentIndexSchema = "" + case "ignored_child": + delete(child.Indexes, childIndex.Name) + case "ignored_parent": + delete(root.Indexes, parentIndex.Name) + case "ignored_table_without_attachments": + parentIndex.AttachedPartitions = nil + delete(desired, "public.events_early") + added = []*ir.Table{root} + case "external_child": + root.AllPartitionChildren = append(root.AllPartitionChildren, ir.TableDependency{Schema: "other", Name: "external_events"}) + case "existing_root": + added = []*ir.Table{child} + case "existing_child": + added = []*ir.Table{root} + case "unmanaged_ancestor": + root.PartitionOf = "outside" + root.PartitionOfSchema = "other" + } + plan := planPartitionIndexCreation(added, desired, false) + if got := plan.contains(parentIndex); got != tc.want { + t.Fatalf("Expected scheduled parent = %v, got %v", tc.want, got) + } + fk := &ir.Constraint{Type: ir.ConstraintTypeForeignKey, ReferencedTable: "events"} + if got := plan.defersForeignKey(&ir.Table{Schema: "public"}, fk); got != tc.want { + t.Fatalf("FK deferral must match scheduled unique indexes: got %v, want %v", got, tc.want) + } + collector := newDiffCollector() + plan.generateSQL("public", collector) + if !tc.want && len(collector.diffs) != 0 { + t.Fatalf("Excluded hierarchy emitted scheduled SQL: %+v", collector.diffs) + } + if tc.name == "intentionally_unattached" && strings.Contains(buildSQLFromSteps(collector.diffs), "ATTACH PARTITION") { + t.Fatal("Unattached child was attached implicitly") + } + }) + } +} diff --git a/internal/diff/qualify_schema_test.go b/internal/diff/qualify_schema_test.go index d1d44c8c..59d46419 100644 --- a/internal/diff/qualify_schema_test.go +++ b/internal/diff/qualify_schema_test.go @@ -60,7 +60,7 @@ func TestQualifySchema_TableAndColumnType(t *testing.T) { } empty := map[string]bool{} - def, _ := generateTableSQL(table, "public", false, empty, empty, empty, nil, false) + def, _ := generateTableSQL(table, "public", false, empty, empty, empty, nil, nil) if !strings.Contains(def, "CREATE TABLE IF NOT EXISTS account (") { t.Errorf("default should use the bare table name: %q", def) } @@ -71,7 +71,7 @@ func TestQualifySchema_TableAndColumnType(t *testing.T) { t.Errorf("default should strip the target-schema prefix from the type ref: %q", def) } - qualified, _ := generateTableSQL(table, "public", true, empty, empty, empty, nil, false) + qualified, _ := generateTableSQL(table, "public", true, empty, empty, empty, nil, 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 2a3f5405..c9fc2e6d 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -487,6 +487,7 @@ func generateCreateTablesSQL( shouldDeferPolicy func(*ir.RLSPolicy) bool, suppressedInlineFKs map[string]bool, allNewTables map[string]*ir.Table, + partitionIndexes *partitionIndexCreationPlan, ) ([]*ir.RLSPolicy, []*deferredConstraint) { var deferredPolicies []*ir.RLSPolicy var deferredConstraints []*deferredConstraint @@ -495,7 +496,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, allNewTables, collector.dump) + sql, tableDeferred := generateTableSQL(table, targetSchema, collector.qualifySchema, createdTables, existingTables, suppressedInlineFKs, allNewTables, partitionIndexes) deferredConstraints = append(deferredConstraints, tableDeferred...) // Create context for this statement @@ -548,7 +549,9 @@ func generateCreateTablesSQL( // Convert map to slice for indexes indexes := make([]*ir.Index, 0, len(table.Indexes)) for _, index := range table.Indexes { - indexes = append(indexes, index) + if !partitionIndexes.contains(index) { + indexes = append(indexes, index) + } } generateCreateIndexesSQL(indexes, targetSchema, collector) @@ -917,7 +920,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, allTables map[string]*ir.Table, dump 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, partitionIndexes *partitionIndexCreationPlan) (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) @@ -988,7 +991,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, continue } if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) || - (dump && referencesPartitionedUniqueIndex(table, constraint, allTables)) { + partitionIndexes.defersForeignKey(table, constraint) { deferred = append(deferred, &deferredConstraint{ table: table, constraint: constraint, @@ -1053,7 +1056,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, continue } if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) || - (dump && referencesPartitionedUniqueIndex(table, constraint, allTables)) { + partitionIndexes.defersForeignKey(table, constraint) { deferred = append(deferred, &deferredConstraint{ table: table, constraint: constraint, diff --git a/ir/index_test.go b/ir/index_test.go index 30921fb3..0f0fcfbd 100644 --- a/ir/index_test.go +++ b/ir/index_test.go @@ -5,18 +5,20 @@ import ( "testing" ) -func TestIndexAttachedPartitionsAreInspectionMetadata(t *testing.T) { +func TestPartitionInspectionMetadataNotSerialized(t *testing.T) { index := &Index{Schema: "public", Table: "events", Name: "event_lookup"} - before, err := json.Marshal(index) + table := &Table{Schema: "public", Name: "events", Indexes: map[string]*Index{index.Name: index}} + before, err := json.Marshal(table) if err != nil { t.Fatal(err) } index.AttachedPartitions = []IndexRef{{Schema: "public", Name: "ignored_child"}} - after, err := json.Marshal(index) + table.AllPartitionChildren = []TableDependency{{Schema: "public", Name: "ignored_partition"}} + after, err := json.Marshal(table) if err != nil { t.Fatal(err) } if string(after) != string(before) { - t.Fatalf("Inspection metadata changed index serialization:\nbefore: %s\nafter: %s", before, after) + t.Fatalf("Inspection metadata changed schema serialization:\nbefore: %s\nafter: %s", before, after) } } diff --git a/ir/inspector.go b/ir/inspector.go index 27cf15bf..734963de 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -752,6 +752,13 @@ func (i *Inspector) populatePartitionChildMetadata(ctx context.Context, schema * dbSchema := schema.getOrCreateSchema(targetSchema) for _, child := range partitionChildren { + if child.ParentSchema == targetSchema { + if parent := dbSchema.Tables[child.ParentTable]; parent != nil { + parent.AllPartitionChildren = append(parent.AllPartitionChildren, TableDependency{ + Schema: child.ChildSchema, Name: child.ChildTable, Type: DependencyTypeTable, + }) + } + } // Only process children in the target schema if child.ChildSchema != targetSchema { continue diff --git a/ir/ir.go b/ir/ir.go index fbbba5ee..60aa37f9 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -80,6 +80,9 @@ type Table struct { // child DEFAULT/NOT NULL overrides without managing or fingerprinting the // parent itself. PartitionParentColumns []*Column `json:"-"` + // AllPartitionChildren records catalog children, including ignored or external + // tables. Creation planning uses this metadata to identify complete hierarchies. + AllPartitionChildren []TableDependency `json:"-"` } // Column represents a table column From 1d258d44c52e34d82a479a6ddc6031e0d9d1c4c3 Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Tue, 13 Oct 2026 12:17:55 +0200 Subject: [PATCH 5/6] fix(diff): preserve constraint-owned index attachments Read catalog attachment edges independently of standalone-index filtering. Keep backing indexes under constraint generation while including them in explicit attachment scheduling and dependency validation. Reject excluded descendants beneath attached constraint-owned indexes before writing dump output. Cover unique and primary-key children, nested constraints, native migrations, and both dump formats with synthetic regressions. --- cmd/dump/partition_index_ignore_test.go | 91 +++++- cmd/dump/partition_index_integration_test.go | 158 +++++----- cmd/partition_index_migration_test.go | 292 ++++++++++--------- cmd/plan/plan.go | 3 + cmd/plan/plan_test.go | 6 + docs/cli/dump.mdx | 3 + docs/cli/ignore.mdx | 9 +- internal/diff/partition_index.go | 95 +++++- internal/diff/partition_index_test.go | 31 ++ ir/index_test.go | 3 + ir/inspector.go | 12 +- ir/ir.go | 3 + ir/partition_index.go | 51 ++++ 13 files changed, 520 insertions(+), 237 deletions(-) create mode 100644 ir/partition_index.go diff --git a/cmd/dump/partition_index_ignore_test.go b/cmd/dump/partition_index_ignore_test.go index 73869ceb..c4f1b1ea 100644 --- a/cmd/dump/partition_index_ignore_test.go +++ b/cmd/dump/partition_index_ignore_test.go @@ -42,14 +42,19 @@ CREATE INDEX late_partial_lookup ON events_late (id); ALTER INDEX partial_lookup ATTACH PARTITION late_partial_lookup; ` for _, tc := range []struct { - name string - ignore string - parent string - child string - dropFK bool - absent string + name string + ignore string + parent string + child string + dropFK bool + absent string + constraintChild bool }{ {name: "complete_family"}, + {name: "constraint_owned_leaf", constraintChild: true}, + {name: "ignored_constraint", constraintChild: true, ignore: "[constraints]\npatterns = [\"late_custom_unique\"]\n", parent: "event_unique", child: "late_custom_unique"}, + {name: "ignored_constraint_table", constraintChild: true, ignore: "[tables]\npatterns = [\"events_late\"]\n", parent: "event_unique", child: "late_custom_unique"}, + {name: "constraint_owned_index_pattern", constraintChild: true, ignore: "[indexes]\npatterns = [\"late_custom_unique\"]\n"}, {name: "ignored_leaf", ignore: "[indexes]\npatterns = [\"late_custom_unique\"]\n", parent: "event_unique", child: "late_custom_unique"}, {name: "ignored_intermediate", ignore: "[indexes]\npatterns = [\"early_custom_unique\"]\n", parent: "event_unique", child: "early_custom_unique"}, {name: "ignored_nested_leaf", ignore: "[indexes]\npatterns = [\"first_custom_unique\"]\n", parent: "early_custom_unique", child: "first_custom_unique"}, @@ -67,7 +72,11 @@ ALTER INDEX partial_lookup ATTACH PARTITION late_partial_lookup; for _, multiFile := range []bool{false, true} { for _, existingOutput := range []bool{false, true} { t.Run(fmt.Sprintf("multi=%v/existing=%v", multiFile, existingOutput), func(t *testing.T) { - if _, err := conn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + setup); err != nil { + fixture := setup + if tc.constraintChild { + fixture = strings.Replace(fixture, "CREATE UNIQUE INDEX late_custom_unique ON events_late (bucket, id);", "ALTER TABLE events_late ADD CONSTRAINT late_custom_unique UNIQUE (bucket, id);", 1) + } + if _, err := conn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + fixture); err != nil { t.Fatal(err) } if tc.dropFK { @@ -133,3 +142,71 @@ ALTER INDEX partial_lookup ATTACH PARTITION late_partial_lookup; }) } } + +// The ignored leaf has only an inherited constraint index. No standalone leaf +// index can cause an unrelated dependency error and mask this regression. +func TestDumpCommand_IgnoredConstraintIndexDescendant(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + t.Chdir(t.TempDir()) + if err := os.WriteFile(".pgschemaignore", []byte("[tables]\npatterns = [\"events_leaf\"]\n"), 0600); err != nil { + t.Fatal(err) + } + source := testutil.SetupPostgres(t) + defer source.Stop() + conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, source) + defer conn.Close() + const setup = ` +CREATE TABLE events (bucket int, id int) PARTITION BY RANGE (bucket); +CREATE TABLE events_mid PARTITION OF events FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (id); +CREATE TABLE events_branch PARTITION OF events_mid FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (bucket); +CREATE TABLE events_leaf PARTITION OF events_branch FOR VALUES FROM (0) TO (10); +ALTER TABLE events_mid ADD CONSTRAINT mid_unique UNIQUE (bucket, id); +CREATE UNIQUE INDEX event_unique ON ONLY events (bucket, id); +ALTER INDEX event_unique ATTACH PARTITION mid_unique; +CREATE TABLE event_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCES events (bucket, id)); +` + if _, err := conn.Exec(setup); err != nil { + t.Fatal(err) + } + for _, multiFile := range []bool{false, true} { + for _, existingOutput := range []bool{false, true} { + t.Run(fmt.Sprintf("multi=%v/existing=%v", multiFile, existingOutput), func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "schema.sql") + const original = "-- preserve existing output\n" + if existingOutput { + if err := os.WriteFile(path, []byte(original), 0600); err != nil { + t.Fatal(err) + } + } + output, err := ExecuteDump(&DumpConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", MultiFile: multiFile, File: path, + }) + if err == nil || !strings.Contains(err.Error(), "event_unique") || !strings.Contains(err.Error(), "mid_unique") || !strings.Contains(err.Error(), "events_leaf") || !strings.Contains(err.Error(), ".pgschemaignore") { + t.Fatalf("Expected actionable nested dependency error, got %v", err) + } + if output != "" { + t.Fatal("Failed dump returned partial SQL") + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + wantFiles := 0 + if existingOutput { + wantFiles = 1 + content, err := os.ReadFile(path) + if err != nil || string(content) != original { + t.Fatalf("Failed dump changed existing output: %q, %v", content, err) + } + } + if len(entries) != wantFiles { + t.Fatalf("Failed dump created files: %v", entries) + } + }) + } + } +} diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go index dec139f5..72afb7bc 100644 --- a/cmd/dump/partition_index_integration_test.go +++ b/cmd/dump/partition_index_integration_test.go @@ -40,7 +40,7 @@ ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup; ALTER INDEX event_lookup ATTACH PARTITION late_custom_lookup; CREATE INDEX early_unattached_lookup ON events_early (label text_pattern_ops DESC NULLS LAST, id) INCLUDE (payload); CREATE UNIQUE INDEX event_unique ON ONLY events (bucket, id); -CREATE UNIQUE INDEX early_custom_unique ON events_early (bucket, id); +ALTER TABLE events_early ADD CONSTRAINT early_custom_unique UNIQUE (bucket, id); CREATE UNIQUE INDEX late_custom_unique ON events_late (bucket, id); ALTER INDEX event_unique ATTACH PARTITION early_custom_unique; ALTER INDEX event_unique ATTACH PARTITION late_custom_unique; @@ -72,95 +72,103 @@ ALTER INDEX z_archive_lookup ATTACH PARTITION first_custom_lookup; ALTER INDEX z_archive_lookup ATTACH PARTITION second_custom_lookup; ALTER INDEX a_archive_lookup ATTACH PARTITION z_archive_lookup; CREATE INDEX first_unattached_lookup ON archive_events_first (label); +ALTER TABLE archive_events_early ADD CONSTRAINT archive_local_unique UNIQUE (bucket, label); +CREATE UNIQUE INDEX archive_unique ON ONLY archive_events (bucket, label); +ALTER INDEX archive_unique ATTACH PARTITION archive_local_unique; +CREATE TABLE archive_links (bucket int, label text, FOREIGN KEY (bucket, label) REFERENCES archive_events (bucket, label)); COMMENT ON INDEX event_lookup IS 'Parent lookup'; COMMENT ON INDEX early_custom_lookup IS 'Named child lookup'; ` - for _, multiFile := range []bool{false, true} { - for _, noComments := range []bool{false, true} { - for _, qualifySchema := range []bool{false, true} { - t.Run(fmt.Sprintf("multi=%v/noComments=%v/qualified=%v", multiFile, noComments, qualifySchema), func(t *testing.T) { - if _, err := conn.ExecContext(ctx, `DROP SCHEMA public CASCADE; CREATE SCHEMA public;`+setup); err != nil { - t.Fatal(err) - } - want := readIndexCatalog(t, conn) - wantFKs := foreignKeyCatalog(t, conn) - path := filepath.Join(t.TempDir(), "schema.sql") - config := &DumpConfig{ - Host: host, Port: port, DB: dbname, User: user, Password: password, - Schema: "public", SSLMode: "disable", MultiFile: multiFile, File: path, - NoComments: noComments, QualifySchema: qualifySchema, - } - output, err := ExecuteDump(config) - if err != nil { - t.Fatal(err) - } - if !multiFile { - if err := os.WriteFile(path, []byte(output), 0600); err != nil { + for _, constraintType := range []string{"UNIQUE", "PRIMARY KEY"} { + for _, multiFile := range []bool{false, true} { + for _, noComments := range []bool{false, true} { + for _, qualifySchema := range []bool{false, true} { + t.Run(fmt.Sprintf("constraint=%s/multi=%v/noComments=%v/qualified=%v", constraintType, multiFile, noComments, qualifySchema), func(t *testing.T) { + fixture := strings.Replace(setup, "early_custom_unique UNIQUE", "early_custom_unique "+constraintType, 1) + if _, err := conn.ExecContext(ctx, `DROP SCHEMA public CASCADE; CREATE SCHEMA public;`+fixture); err != nil { t.Fatal(err) } - } - // Compare the dump with the independent source BEFORE replay changes it. - migration, err := plan.GeneratePlan(&plan.PlanConfig{ - Host: host, Port: port, DB: dbname, User: user, Password: password, - Schema: "public", SSLMode: "disable", File: path, - }, provider) - if err != nil { - t.Errorf("Plan against independent source failed: %v", err) - } else if len(migration.Groups) != 0 { - t.Errorf("Expected empty plan against independent source, got: %+v", migration.Groups) - } - sqlText, err := include.NewProcessor(filepath.Dir(path)).ProcessFile(path) - if err != nil { - t.Fatal(err) - } - prefix := "" - if qualifySchema { - prefix = "public." - } - attachment := "ALTER INDEX " + prefix + "event_lookup ATTACH PARTITION " + prefix + "early_custom_lookup;" - if !strings.Contains(sqlText, attachment) { - t.Errorf("Missing explicit attachment: %s", attachment) - } - if strings.Contains(sqlText, "-- Name:") == noComments { - t.Error("Unexpected object header formatting") - } - mainContent, err := os.ReadFile(path) - if err != nil { - t.Fatal(err) - } - repeated, err := ExecuteDump(config) - if err != nil { - t.Fatal(err) - } - if multiFile { - repeatedMain, err := os.ReadFile(path) + want := readIndexCatalog(t, conn) + wantFKs := foreignKeyCatalog(t, conn) + path := filepath.Join(t.TempDir(), "schema.sql") + config := &DumpConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", MultiFile: multiFile, File: path, + NoComments: noComments, QualifySchema: qualifySchema, + } + output, err := ExecuteDump(config) if err != nil { t.Fatal(err) } - if string(repeatedMain) != string(mainContent) { - t.Error("Non-deterministic include order or filenames") + if !multiFile { + if err := os.WriteFile(path, []byte(output), 0600); err != nil { + t.Fatal(err) + } + } + // Compare the dump with the independent source BEFORE replay changes it. + migration, err := plan.GeneratePlan(&plan.PlanConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + }, provider) + if err != nil { + t.Errorf("Plan against independent source failed: %v", err) + } else if len(migration.Groups) != 0 { + t.Errorf("Expected empty plan against independent source, got: %+v", migration.Groups) + } + sqlText, err := include.NewProcessor(filepath.Dir(path)).ProcessFile(path) + if err != nil { + t.Fatal(err) + } + prefix := "" + if qualifySchema { + prefix = "public." + } + attachment := "ALTER INDEX " + prefix + "event_lookup ATTACH PARTITION " + prefix + "early_custom_lookup;" + if !strings.Contains(sqlText, attachment) { + t.Errorf("Missing explicit attachment: %s", attachment) + } + if strings.Contains(sqlText, "-- Name:") == noComments { + t.Error("Unexpected object header formatting") } - repeated, err = include.NewProcessor(filepath.Dir(path)).ProcessFile(path) + mainContent, err := os.ReadFile(path) if err != nil { t.Fatal(err) } - } - if repeated != sqlText { - t.Error("Non-deterministic SQL output") - } - if _, err := conn.ExecContext(ctx, `DROP SCHEMA public CASCADE; CREATE SCHEMA public;`+sqlText); err != nil { - t.Fatalf("Dump replay failed: %v", err) - } - if got := readIndexCatalog(t, conn); got != want { - t.Errorf("Index catalog changed after replay:\ngot: %s\nwant: %s", got, want) - } - if got := foreignKeyCatalog(t, conn); !reflect.DeepEqual(got, wantFKs) { - t.Errorf("Foreign keys changed after replay: got %v, want %v", got, wantFKs) - } - }) + repeated, err := ExecuteDump(config) + if err != nil { + t.Fatal(err) + } + if multiFile { + repeatedMain, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(repeatedMain) != string(mainContent) { + t.Error("Non-deterministic include order or filenames") + } + repeated, err = include.NewProcessor(filepath.Dir(path)).ProcessFile(path) + if err != nil { + t.Fatal(err) + } + } + if repeated != sqlText { + t.Error("Non-deterministic SQL output") + } + if _, err := conn.ExecContext(ctx, `DROP SCHEMA public CASCADE; CREATE SCHEMA public;`+sqlText); err != nil { + t.Fatalf("Dump replay failed: %v", err) + } + if got := readIndexCatalog(t, conn); got != want { + t.Errorf("Index catalog changed after replay:\ngot: %s\nwant: %s", got, want) + } + if got := foreignKeyCatalog(t, conn); !reflect.DeepEqual(got, wantFKs) { + t.Errorf("Foreign keys changed after replay: got %v, want %v", got, wantFKs) + } + }) + } } } } + } func readIndexCatalog(t *testing.T, conn *sql.DB) string { diff --git a/cmd/partition_index_migration_test.go b/cmd/partition_index_migration_test.go index 59915f01..a2763970 100644 --- a/cmd/partition_index_migration_test.go +++ b/cmd/partition_index_migration_test.go @@ -28,7 +28,7 @@ func TestPlanAndApplyPartitionIndexIdentity(t *testing.T) { targetConn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, target) defer targetConn.Close() - const desired = ` + const schemaSQL = ` CREATE TABLE events (bucket int, id int, label text) PARTITION BY RANGE (bucket); CREATE TABLE events_early PARTITION OF events FOR VALUES FROM (0) TO (10); CREATE TABLE events_late PARTITION OF events FOR VALUES FROM (10) TO (20); @@ -39,7 +39,7 @@ ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup; ALTER INDEX event_lookup ATTACH PARTITION late_custom_lookup; CREATE INDEX early_unattached_lookup ON events_early (label); CREATE UNIQUE INDEX event_unique ON ONLY events (bucket, id); -CREATE UNIQUE INDEX early_custom_unique ON events_early (bucket, id); +ALTER TABLE events_early ADD CONSTRAINT early_custom_unique UNIQUE (bucket, id); CREATE UNIQUE INDEX late_custom_unique ON events_late (bucket, id); ALTER INDEX event_unique ATTACH PARTITION early_custom_unique; ALTER INDEX event_unique ATTACH PARTITION late_custom_unique; @@ -60,6 +60,10 @@ ALTER INDEX z_archive_lookup ATTACH PARTITION first_custom_lookup; ALTER INDEX z_archive_lookup ATTACH PARTITION second_custom_lookup; ALTER INDEX a_archive_lookup ATTACH PARTITION z_archive_lookup; CREATE INDEX first_unattached_lookup ON archive_events_first (label); +ALTER TABLE archive_events_early ADD CONSTRAINT archive_local_unique UNIQUE (bucket, label); +CREATE UNIQUE INDEX archive_unique ON ONLY archive_events (bucket, label); +ALTER INDEX archive_unique ATTACH PARTITION archive_local_unique; +CREATE TABLE archive_links (bucket int, label text, FOREIGN KEY (bucket, label) REFERENCES archive_events (bucket, label)); CREATE INDEX "Event.Lookup" ON ONLY events (id DESC); CREATE INDEX "Early.Lookup" ON events_early (id DESC); CREATE INDEX "Late.Lookup" ON events_late (id DESC); @@ -70,152 +74,156 @@ CREATE INDEX empty_lookup ON empty_events (id); COMMENT ON INDEX event_lookup IS 'Parent lookup'; COMMENT ON INDEX early_custom_lookup IS 'Named child lookup'; ` - for _, initial := range []struct{ name, sql, desiredPrefix string }{ - {name: "empty"}, - {name: "unrelated_table", sql: "CREATE TABLE unrelated (id integer);\n", desiredPrefix: "CREATE TABLE unrelated (id integer);\n"}, - {name: "existing_referencing_table", sql: "CREATE TABLE event_links (bucket int, id int);\n"}, - } { - for _, executor := range []string{"native", "sql"} { - t.Run(initial.name+"/"+executor, func(t *testing.T) { - if _, err := sourceConn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + initial.desiredPrefix + desired); err != nil { - t.Fatal(err) - } - wantCount, wantCatalog := migrationIndexCatalog(t, sourceConn) - if wantCount != 19 { - t.Fatalf("Expected 19 source indexes, got %d", wantCount) - } - if _, err := targetConn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + initial.sql); err != nil { - t.Fatal(err) - } - path := filepath.Join(t.TempDir(), "schema.sql") - if err := os.WriteFile(path, []byte(initial.desiredPrefix+desired), 0600); err != nil { - t.Fatal(err) - } - config := &planCmd.PlanConfig{ - Host: host, Port: port, DB: dbname, User: user, Password: password, - Schema: "public", SSLMode: "disable", File: path, - } - migration, err := planCmd.GeneratePlan(config, sharedEmbeddedPG) - if err != nil { - t.Fatal(err) - } - var statements []string - for _, group := range migration.Groups { - for _, step := range group.Steps { - if step.SQL != "" { - statements = append(statements, step.SQL) + for _, constraintType := range []string{"UNIQUE", "PRIMARY KEY"} { + desired := strings.Replace(schemaSQL, "early_custom_unique UNIQUE", "early_custom_unique "+constraintType, 1) + for _, initial := range []struct{ name, sql, desiredPrefix string }{ + {name: "empty"}, + {name: "unrelated_table", sql: "CREATE TABLE unrelated (id integer);\n", desiredPrefix: "CREATE TABLE unrelated (id integer);\n"}, + {name: "existing_referencing_table", sql: "CREATE TABLE event_links (bucket int, id int);\n"}, + } { + for _, executor := range []string{"native", "sql"} { + t.Run(constraintType+"/"+initial.name+"/"+executor, func(t *testing.T) { + if _, err := sourceConn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + initial.desiredPrefix + desired); err != nil { + t.Fatal(err) + } + wantCount, wantCatalog := migrationIndexCatalog(t, sourceConn) + if wantCount != 23 { + t.Fatalf("Expected 23 source indexes, got %d", wantCount) + } + if _, err := targetConn.Exec(`DROP SCHEMA public CASCADE; CREATE SCHEMA public;` + initial.sql); err != nil { + t.Fatal(err) + } + path := filepath.Join(t.TempDir(), "schema.sql") + if err := os.WriteFile(path, []byte(initial.desiredPrefix+desired), 0600); err != nil { + t.Fatal(err) + } + config := &planCmd.PlanConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + } + migration, err := planCmd.GeneratePlan(config, sharedEmbeddedPG) + if err != nil { + t.Fatal(err) + } + var statements []string + for _, group := range migration.Groups { + for _, step := range group.Steps { + if step.SQL != "" { + statements = append(statements, step.SQL) + } } } - } - generatedSQL := strings.Join(statements, "\n") - if !strings.Contains(generatedSQL, "event_lookup ON ONLY events") || - !strings.Contains(generatedSQL, "ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup") { - t.Errorf("Generated migration does not preserve explicit index attachments:\n%s", generatedSQL) - } - if strings.Contains(generatedSQL, "CONCURRENTLY") { - t.Errorf("New-table index creation must remain synchronous:\n%s", generatedSQL) - } - if initial.name == "empty" && executor == "native" { - if len(migration.Groups) != 1 { - t.Fatalf("Expected one transactional bootstrap group, got %d", len(migration.Groups)) + generatedSQL := strings.Join(statements, "\n") + if !strings.Contains(generatedSQL, "event_lookup ON ONLY events") || + !strings.Contains(generatedSQL, "ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup") { + t.Errorf("Generated migration does not preserve explicit index attachments:\n%s", generatedSQL) } - // A failed attachment must propagate without a recursive fallback. - // PostgreSQL must roll back the ordinary group before a retry. - failedAttachment := false - for i := range migration.Groups[0].Steps { - step := &migration.Groups[0].Steps[i] - if step.SQL != "ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup;" { - continue + if strings.Contains(generatedSQL, "CONCURRENTLY") { + t.Errorf("New-table index creation must remain synchronous:\n%s", generatedSQL) + } + if initial.name == "empty" && executor == "native" { + if len(migration.Groups) != 1 { + t.Fatalf("Expected one transactional bootstrap group, got %d", len(migration.Groups)) + } + // A failed attachment must propagate without a recursive fallback. + // PostgreSQL must roll back the ordinary group before a retry. + failedAttachment := false + for i := range migration.Groups[0].Steps { + step := &migration.Groups[0].Steps[i] + if step.SQL != "ALTER INDEX event_lookup ATTACH PARTITION early_custom_lookup;" { + continue + } + originalSQL := step.SQL + step.SQL = "ALTER INDEX event_lookup ATTACH PARTITION missing_lookup;" + applyErr := apply.ApplyMigration(&apply.ApplyConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", Plan: migration, + AutoApprove: true, NoColor: true, Quiet: true, + }, nil) + step.SQL = originalSQL + if applyErr == nil || !strings.Contains(applyErr.Error(), "missing_lookup") { + t.Fatalf("Expected attachment failure, got %v", applyErr) + } + var tableCount int + if err := targetConn.QueryRow(`SELECT count(*) FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = 'public' AND c.relkind IN ('r', 'p')`).Scan(&tableCount); err != nil { + t.Fatal(err) + } + if count, _ := migrationIndexCatalog(t, targetConn); count != 0 || tableCount != 0 { + t.Fatalf("Failed group left %d tables and %d indexes", tableCount, count) + } + failedAttachment = true + break } - originalSQL := step.SQL - step.SQL = "ALTER INDEX event_lookup ATTACH PARTITION missing_lookup;" - applyErr := apply.ApplyMigration(&apply.ApplyConfig{ + if !failedAttachment { + t.Fatal("No explicit attachment available for failure test") + } + } + if executor == "native" { + err = apply.ApplyMigration(&apply.ApplyConfig{ Host: host, Port: port, DB: dbname, User: user, Password: password, Schema: "public", SSLMode: "disable", Plan: migration, AutoApprove: true, NoColor: true, Quiet: true, }, nil) - step.SQL = originalSQL - if applyErr == nil || !strings.Contains(applyErr.Error(), "missing_lookup") { - t.Fatalf("Expected attachment failure, got %v", applyErr) - } - var tableCount int - if err := targetConn.QueryRow(`SELECT count(*) FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = 'public' AND c.relkind IN ('r', 'p')`).Scan(&tableCount); err != nil { - t.Fatal(err) - } - if count, _ := migrationIndexCatalog(t, targetConn); count != 0 || tableCount != 0 { - t.Fatalf("Failed group left %d tables and %d indexes", tableCount, count) + } else { + // Use the emitted SQL directly to isolate generation from the executor. + for _, statement := range statements { + if _, err = targetConn.Exec(statement); err != nil { + break + } } - failedAttachment = true - break } - if !failedAttachment { - t.Fatal("No explicit attachment available for failure test") + if err != nil { + t.Fatalf("Migration failed: %v", err) } - } - if executor == "native" { - err = apply.ApplyMigration(&apply.ApplyConfig{ - Host: host, Port: port, DB: dbname, User: user, Password: password, - Schema: "public", SSLMode: "disable", Plan: migration, - AutoApprove: true, NoColor: true, Quiet: true, - }, nil) - } else { - // Use the emitted SQL directly to isolate generation from the executor. - for _, statement := range statements { - if _, err = targetConn.Exec(statement); err != nil { - break - } + gotCount, gotCatalog := migrationIndexCatalog(t, targetConn) + if gotCatalog != wantCatalog { + t.Errorf("Index catalog mismatch: expected %d indexes, got %d\nwant: %s\ngot: %s", wantCount, gotCount, wantCatalog, gotCatalog) + } + if got, want := migrationFKCatalog(t, targetConn), migrationFKCatalog(t, sourceConn); got != want { + t.Errorf("FK catalog mismatch:\nwant: %s\ngot: %s", want, got) + } + followUp, err := planCmd.GeneratePlan(config, sharedEmbeddedPG) + if err != nil { + t.Fatal(err) + } + if len(followUp.Groups) != 0 { + t.Errorf("Expected empty follow-up plan, got %+v", followUp.Groups) } - } - if err != nil { - t.Fatalf("Migration failed: %v", err) - } - gotCount, gotCatalog := migrationIndexCatalog(t, targetConn) - if gotCatalog != wantCatalog { - t.Errorf("Index catalog mismatch: expected %d indexes, got %d\nwant: %s\ngot: %s", wantCount, gotCount, wantCatalog, gotCatalog) - } - if got, want := migrationFKCatalog(t, targetConn), migrationFKCatalog(t, sourceConn); got != want { - t.Errorf("FK catalog mismatch:\nwant: %s\ngot: %s", want, got) - } - followUp, err := planCmd.GeneratePlan(config, sharedEmbeddedPG) - if err != nil { - t.Fatal(err) - } - if len(followUp.Groups) != 0 { - t.Errorf("Expected empty follow-up plan, got %+v", followUp.Groups) - } - // Keep incremental creation under an existing parent and unrelated - // table alterations covered after the independent bootstrap check. - const incremental = `ALTER TABLE event_links ADD COLUMN note text; + // Keep incremental creation under an existing parent and unrelated + // table alterations covered after the independent bootstrap check. + const incremental = `ALTER TABLE event_links ADD COLUMN note text; CREATE TABLE events_future PARTITION OF events FOR VALUES FROM (20) TO (30);` - if _, err := sourceConn.Exec(incremental); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(path, []byte(initial.desiredPrefix+desired+incremental), 0600); err != nil { - t.Fatal(err) - } - if err := apply.ApplyMigration(&apply.ApplyConfig{ - Host: host, Port: port, DB: dbname, User: user, Password: password, - Schema: "public", SSLMode: "disable", File: path, - AutoApprove: true, NoColor: true, Quiet: true, - }, sharedEmbeddedPG); err != nil { - t.Fatalf("Incremental migration failed: %v", err) - } - _, wantCatalog = migrationIndexCatalog(t, sourceConn) - _, gotCatalog = migrationIndexCatalog(t, targetConn) - if gotCatalog != wantCatalog { - t.Errorf("Incremental migration changed index identity:\nwant: %s\ngot: %s", wantCatalog, gotCatalog) - } - if got, want := migrationFKCatalog(t, targetConn), migrationFKCatalog(t, sourceConn); got != want { - t.Errorf("Incremental migration changed FKs:\nwant: %s\ngot: %s", want, got) - } - followUp, err = planCmd.GeneratePlan(config, sharedEmbeddedPG) - if err != nil || len(followUp.Groups) != 0 { - t.Fatalf("Incremental follow-up plan is not empty: %+v, %v", followUp, err) - } - }) + if _, err := sourceConn.Exec(incremental); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(initial.desiredPrefix+desired+incremental), 0600); err != nil { + t.Fatal(err) + } + if err := apply.ApplyMigration(&apply.ApplyConfig{ + Host: host, Port: port, DB: dbname, User: user, Password: password, + Schema: "public", SSLMode: "disable", File: path, + AutoApprove: true, NoColor: true, Quiet: true, + }, sharedEmbeddedPG); err != nil { + t.Fatalf("Incremental migration failed: %v", err) + } + _, wantCatalog = migrationIndexCatalog(t, sourceConn) + _, gotCatalog = migrationIndexCatalog(t, targetConn) + if gotCatalog != wantCatalog { + t.Errorf("Incremental migration changed index identity:\nwant: %s\ngot: %s", wantCatalog, gotCatalog) + } + if got, want := migrationFKCatalog(t, targetConn), migrationFKCatalog(t, sourceConn); got != want { + t.Errorf("Incremental migration changed FKs:\nwant: %s\ngot: %s", want, got) + } + followUp, err = planCmd.GeneratePlan(config, sharedEmbeddedPG) + if err != nil || len(followUp.Groups) != 0 { + t.Fatalf("Incremental follow-up plan is not empty: %+v, %v", followUp, err) + } + }) + } } } + } func migrationIndexCatalog(t *testing.T, conn *sql.DB) (int, string) { @@ -278,10 +286,14 @@ CREATE INDEX parent_lookup ON ONLY events (label); CREATE INDEX custom_lookup ON events_early (label); ALTER INDEX parent_lookup ATTACH PARTITION custom_lookup; ` - for _, tc := range []struct{ name, ignore string }{ + for _, tc := range []struct { + name, ignore string + constraintChild bool + }{ {name: "ignored_attached_index", ignore: "[indexes]\npatterns = [\"custom_lookup\"]\n"}, {name: "ignored_attached_table", ignore: "[tables]\npatterns = [\"events_early\"]\n"}, {name: "ignored_table_without_index", ignore: "[tables]\npatterns = [\"events_late\"]\n"}, + {name: "ignored_owning_constraint", ignore: "[constraints]\npatterns = [\"custom_lookup\"]\n", constraintChild: true}, } { t.Run(tc.name, func(t *testing.T) { t.Chdir(t.TempDir()) @@ -289,7 +301,17 @@ ALTER INDEX parent_lookup ATTACH PARTITION custom_lookup; t.Fatal(err) } path := filepath.Join(t.TempDir(), "schema.sql") - if err := os.WriteFile(path, []byte(desired), 0600); err != nil { + fixture := desired + indexPrefix := "CREATE INDEX" + indexColumns := "(label)" + if tc.constraintChild { + fixture = strings.Replace(fixture, "CREATE INDEX parent_lookup", "CREATE UNIQUE INDEX parent_lookup", 1) + fixture = strings.Replace(fixture, "CREATE INDEX custom_lookup ON events_early (label);", "ALTER TABLE events_early ADD CONSTRAINT custom_lookup UNIQUE (label);", 1) + indexPrefix = "CREATE UNIQUE INDEX" + indexColumns = "(id, label)" + fixture = strings.ReplaceAll(fixture, "(label)", indexColumns) + } + if err := os.WriteFile(path, []byte(fixture), 0600); err != nil { t.Fatal(err) } migration, err := planCmd.GeneratePlan(&planCmd.PlanConfig{ @@ -306,7 +328,7 @@ ALTER INDEX parent_lookup ATTACH PARTITION custom_lookup; } } text := sqlText.String() - if !strings.Contains(text, "CREATE INDEX IF NOT EXISTS parent_lookup ON events (label);") || strings.Contains(text, "ATTACH PARTITION") { + if !strings.Contains(text, indexPrefix+" IF NOT EXISTS parent_lookup ON events "+indexColumns+";") || strings.Contains(text, "ATTACH PARTITION") { t.Fatalf("Filtered hierarchy must retain recursive migration behavior:\n%s", text) } }) diff --git a/cmd/plan/plan.go b/cmd/plan/plan.go index c58a0354..44a1e00a 100644 --- a/cmd/plan/plan.go +++ b/cmd/plan/plan.go @@ -527,6 +527,9 @@ func normalizeSchemaNames(irData *ir.IR, fromSchema, toSchema string) { // Normalize constraint schemas for _, constraint := range table.Constraints { + if constraint.BackingIndex != nil && constraint.BackingIndex.Schema == fromSchema { + constraint.BackingIndex.Schema = toSchema + } // Normalize the constraint's own schema field if constraint.Schema == fromSchema { constraint.Schema = toSchema diff --git a/cmd/plan/plan_test.go b/cmd/plan/plan_test.go index 4cec9bad..6569f3be 100644 --- a/cmd/plan/plan_test.go +++ b/cmd/plan/plan_test.go @@ -296,9 +296,12 @@ func TestNormalizeSchemaNames_IndexAttachment(t *testing.T) { AttachedPartitions: []ir.IndexRef{{Schema: temporary, Name: "leaf"}, {Schema: "other", Name: "outside_leaf"}}, } external := &ir.Index{Schema: temporary, ParentIndex: "outside_parent", ParentIndexSchema: "other"} + localConstraint := &ir.Constraint{BackingIndex: &ir.IndexRef{Schema: temporary, Name: "local_constraint"}} + externalConstraint := &ir.Constraint{BackingIndex: &ir.IndexRef{Schema: "other", Name: "external_constraint"}} data := &ir.IR{Schemas: map[string]*ir.Schema{ temporary: {Name: temporary, Tables: map[string]*ir.Table{ "child": {Schema: temporary, Name: "child", Indexes: map[string]*ir.Index{"local": local, "external": external}, + Constraints: map[string]*ir.Constraint{"local": localConstraint, "external": externalConstraint}, AllPartitionChildren: []ir.TableDependency{{Schema: temporary, Name: "leaf"}, {Schema: "other", Name: "outside_leaf"}}, }, }}, @@ -313,6 +316,9 @@ func TestNormalizeSchemaNames_IndexAttachment(t *testing.T) { if local.AttachedPartitions[0].Schema != "public" || local.AttachedPartitions[1].Schema != "other" { t.Fatalf("Incorrect attached-index metadata normalization: %+v", local.AttachedPartitions) } + if localConstraint.BackingIndex.Schema != "public" || externalConstraint.BackingIndex.Schema != "other" { + t.Fatalf("Incorrect constraint-index normalization: %+v, %+v", localConstraint.BackingIndex, externalConstraint.BackingIndex) + } children := data.Schemas["public"].Tables["child"].AllPartitionChildren if children[0].Schema != "public" || children[1].Schema != "other" { t.Fatalf("Incorrect partition-table metadata normalization: %+v", children) diff --git a/docs/cli/dump.mdx b/docs/cli/dump.mdx index 207ea64d..2c7de731 100644 --- a/docs/cli/dump.mdx +++ b/docs/cli/dump.mdx @@ -260,6 +260,9 @@ For partitioned indexes, `pgschema` creates the tables and leaf indexes first. The exporter then creates each parent index with `ON ONLY` and attaches its recorded child indexes with `ALTER INDEX ... ATTACH PARTITION`. This preserves custom child names and leaves intentionally unattached indexes separate. +If a child index backs a `UNIQUE` or `PRIMARY KEY` constraint, the constraint +generator creates that index. The exporter then attaches it to its recorded +parent index. Multi-file dumps put parent indexes and their attachments under `indexes/`, for example `indexes/0001_events_lookup.sql`, after the table includes. diff --git a/docs/cli/ignore.mdx b/docs/cli/ignore.mdx index 9d354ff6..3af60109 100644 --- a/docs/cli/ignore.mdx +++ b/docs/cli/ignore.mdx @@ -122,8 +122,8 @@ The `[privileges]` section filters explicit grants (`GRANT ... TO role`), includ ## Partitioned indexes When `dump` retains a partitioned index, it must also retain that index's attached -children. If the inspector excludes an attached child because of an index or table -ignore pattern, `dump` returns an error naming the parent and child before writing +children. If the inspector excludes an attached child because of an index, table, +or owning constraint ignore pattern, `dump` returns an error naming the parent and child before writing any output. Existing output files remain unchanged. `dump` performs the same check for nested partitions. @@ -133,6 +133,11 @@ an error. Remove the conflicting ignore pattern or add a negation pattern to retain the child index and its table. Ignoring an unattached index remains supported. +For constraint-backed child indexes, use `[constraints]` to ignore the owning +constraint. An `[indexes]` pattern does not exclude a constraint-backed index. +For example, if `early_custom_unique` is a `UNIQUE` constraint, excluding it under +`[constraints]` while retaining its parent index causes `dump` to reject the dump. + `plan` and `apply` retain their existing ignore behavior. ## Constraints diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go index 8cd3b1a8..c51c5753 100644 --- a/internal/diff/partition_index.go +++ b/internal/diff/partition_index.go @@ -7,13 +7,40 @@ import ( "github.com/pgplex/pgschema/ir" ) +// availablePartitionIndexes includes indexes created by retained constraints. +// Those indexes remain outside standalone index creation and comparison. +func availablePartitionIndexes(tables []*ir.Table) map[ir.IndexRef]bool { + available := make(map[ir.IndexRef]bool) + for _, table := range tables { + for _, index := range table.Indexes { + available[ir.IndexRef{Schema: index.Schema, Name: index.Name}] = true + } + for _, constraint := range table.Constraints { + if constraint.BackingIndex != nil { + available[*constraint.BackingIndex] = true + } + } + } + return available +} + // validatePartitionIndexDependencies rejects filtered index families before any // dump statements are generated. Migration generation still honors filtering. func validatePartitionIndexDependencies(schema *ir.IR) error { indexes := make(map[ir.IndexRef]*ir.Index) var keys []ir.IndexRef + var tables []*ir.Table + tablesByName := make(map[string]*ir.Table) + constraintIndexes := make(map[ir.IndexRef]*ir.Table) for _, namespace := range schema.Schemas { for _, table := range namespace.Tables { + tables = append(tables, table) + tablesByName[table.Schema+"."+table.Name] = table + for _, constraint := range table.Constraints { + if constraint.BackingIndex != nil { + constraintIndexes[*constraint.BackingIndex] = table + } + } for _, index := range table.Indexes { key := ir.IndexRef{Schema: index.Schema, Name: index.Name} indexes[key] = index @@ -27,13 +54,47 @@ func validatePartitionIndexDependencies(schema *ir.IR) error { } return keys[i].Name < keys[j].Name }) + available := availablePartitionIndexes(tables) for _, key := range keys { for _, child := range indexes[key].AttachedPartitions { - if _, exists := indexes[child]; !exists { - return fmt.Errorf("cannot dump index %s: attached index %s is excluded; adjust .pgschemaignore to include the child index and its table", + if !available[child] { + return fmt.Errorf("cannot dump index %s: attached index %s is excluded; adjust .pgschemaignore to include the child index, its table, and any owning constraint", ir.QuoteIdentifier(key.Schema)+"."+ir.QuoteIdentifier(key.Name), ir.QuoteIdentifier(child.Schema)+"."+ir.QuoteIdentifier(child.Name)) } + if table := constraintIndexes[child]; table != nil { + if err := validateConstraintIndexPartitions(key, child, table, tablesByName); err != nil { + return err + } + } + } + } + return nil +} + +// Constraint generation recursively creates an index on every table partition. +// An attached constraint-owned index therefore requires all of those tables, +// including descendants whose inherited constraints are not separate IR objects. +func validateConstraintIndexPartitions(parent, child ir.IndexRef, table *ir.Table, tables map[string]*ir.Table) error { + pending := []*ir.Table{table} + seen := make(map[string]bool) + for len(pending) > 0 { + current := pending[len(pending)-1] + pending = pending[:len(pending)-1] + key := current.Schema + "." + current.Name + if seen[key] { + continue + } + seen[key] = true + for _, partition := range current.AllPartitionChildren { + next := tables[partition.Schema+"."+partition.Name] + if next == nil || next.IsExternal { + return fmt.Errorf("cannot dump index %s: attached constraint-backed index %s requires excluded partition table %s; adjust .pgschemaignore to include the partition table", + ir.QuoteIdentifier(parent.Schema)+"."+ir.QuoteIdentifier(parent.Name), + ir.QuoteIdentifier(child.Schema)+"."+ir.QuoteIdentifier(child.Name), + ir.QuoteIdentifier(partition.Schema)+"."+ir.QuoteIdentifier(partition.Name)) + } + pending = append(pending, next) } } return nil @@ -46,17 +107,18 @@ func validatePartitionIndexDependencies(schema *ir.IR) error { // Dump selects every managed partitioned index after its strict validation. type partitionIndexCreationPlan struct { indexes map[ir.IndexRef]*ir.Index - children map[ir.IndexRef][]*ir.Index + children map[ir.IndexRef]map[ir.IndexRef]bool uniqueTables map[string]bool } func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[string]*ir.Table, dump bool) *partitionIndexCreationPlan { plan := &partitionIndexCreationPlan{ indexes: make(map[ir.IndexRef]*ir.Index), - children: make(map[ir.IndexRef][]*ir.Index), + children: make(map[ir.IndexRef]map[ir.IndexRef]bool), uniqueTables: make(map[string]bool), } added := make(map[string]*ir.Table) + available := availablePartitionIndexes(addedTables) indexes := make(map[ir.IndexRef]*ir.Index) for _, table := range addedTables { added[table.Schema+"."+table.Name] = table @@ -88,11 +150,11 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin } for _, index := range table.Indexes { for _, child := range index.AttachedPartitions { - if indexes[child] == nil { + if !available[child] { blocked[root] = true } } - if index.ParentIndex != "" && indexes[ir.IndexRef{Schema: index.ParentIndexSchema, Name: index.ParentIndex}] == nil { + if index.ParentIndex != "" && !available[ir.IndexRef{Schema: index.ParentIndexSchema, Name: index.ParentIndex}] { blocked[root] = true } } @@ -113,10 +175,18 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin } } } + for parent, index := range plan.indexes { + plan.children[parent] = make(map[ir.IndexRef]bool) + for _, child := range index.AttachedPartitions { + plan.children[parent][child] = true + } + } + // ParentIndex also preserves standalone edges in IR without transient + // inspection metadata, such as an IR restored from JSON. for _, index := range indexes { parent := ir.IndexRef{Schema: index.ParentIndexSchema, Name: index.ParentIndex} - if plan.indexes[parent] != nil { - plan.children[parent] = append(plan.children[parent], index) + if children := plan.children[parent]; children != nil { + children[ir.IndexRef{Schema: index.Schema, Name: index.Name}] = true } } return plan @@ -181,7 +251,10 @@ func (p *partitionIndexCreationPlan) generateSQL(targetSchema string, collector return } created[key] = true - attached := p.children[key] + attached := make([]ir.IndexRef, 0, len(p.children[key])) + for child := range p.children[key] { + attached = append(attached, child) + } sort.Slice(attached, func(i, j int) bool { if attached[i].Schema != attached[j].Schema { return attached[i].Schema < attached[j].Schema @@ -189,8 +262,8 @@ func (p *partitionIndexCreationPlan) generateSQL(targetSchema string, collector return attached[i].Name < attached[j].Name }) for _, child := range attached { - if child.IsPartitioned { - create(ir.IndexRef{Schema: child.Schema, Name: child.Name}) + if p.indexes[child] != nil { + create(child) } } index := p.indexes[key] diff --git a/internal/diff/partition_index_test.go b/internal/diff/partition_index_test.go index 72d3d85e..c8bcc8f9 100644 --- a/internal/diff/partition_index_test.go +++ b/internal/diff/partition_index_test.go @@ -124,6 +124,9 @@ func TestPartitionIndexCreationEligibility(t *testing.T) { want bool }{ {name: "complete", want: true}, + {name: "constraint_owned_child", want: true}, + {name: "ignored_constraint"}, + {name: "serialized_standalone_edges", want: true}, {name: "unrelated_existing_hierarchy", want: true}, {name: "intentionally_unattached", want: true}, {name: "ignored_child"}, @@ -145,6 +148,17 @@ func TestPartitionIndexCreationEligibility(t *testing.T) { desired := map[string]*ir.Table{"public.events": root, "public.events_early": child} added := []*ir.Table{root, child} switch tc.name { + case "constraint_owned_child", "ignored_constraint": + delete(child.Indexes, childIndex.Name) + if tc.name == "constraint_owned_child" { + child.Constraints = map[string]*ir.Constraint{childIndex.Name: { + Schema: child.Schema, Table: child.Name, Name: childIndex.Name, Type: ir.ConstraintTypeUnique, + BackingIndex: &ir.IndexRef{Schema: child.Schema, Name: childIndex.Name}, + Columns: []*ir.ConstraintColumn{{Name: "id", Position: 1}}, + }} + } + case "serialized_standalone_edges": + parentIndex.AttachedPartitions = nil case "unrelated_existing_hierarchy": desired["public.old_events"] = &ir.Table{Schema: "public", Name: "old_events", IsPartitioned: true} case "intentionally_unattached": @@ -179,6 +193,23 @@ func TestPartitionIndexCreationEligibility(t *testing.T) { } collector := newDiffCollector() plan.generateSQL("public", collector) + if tc.name == "constraint_owned_child" || tc.name == "serialized_standalone_edges" { + if strings.Count(buildSQLFromSteps(collector.diffs), "ATTACH PARTITION custom_lookup;") != 1 { + t.Fatal("Expected exactly one attachment to the existing child index") + } + } + if tc.name == "constraint_owned_child" || tc.name == "ignored_constraint" { + diffs, err := GenerateDump(schema, "public", false) + if (err == nil) != tc.want { + t.Fatalf("Dump validation must follow constraint ownership: %v", err) + } + if tc.want { + sqlText := buildSQLFromSteps(diffs) + if !strings.Contains(sqlText, "CONSTRAINT custom_lookup UNIQUE") || strings.Contains(sqlText, "CREATE UNIQUE INDEX IF NOT EXISTS custom_lookup") { + t.Fatalf("Child index must be created by its constraint:\n%s", sqlText) + } + } + } if !tc.want && len(collector.diffs) != 0 { t.Fatalf("Excluded hierarchy emitted scheduled SQL: %+v", collector.diffs) } diff --git a/ir/index_test.go b/ir/index_test.go index 0f0fcfbd..2cf1cc01 100644 --- a/ir/index_test.go +++ b/ir/index_test.go @@ -8,11 +8,14 @@ import ( func TestPartitionInspectionMetadataNotSerialized(t *testing.T) { index := &Index{Schema: "public", Table: "events", Name: "event_lookup"} table := &Table{Schema: "public", Name: "events", Indexes: map[string]*Index{index.Name: index}} + constraint := &Constraint{Schema: "public", Table: "events", Name: "unique_constraint", Type: ConstraintTypeUnique} + table.Constraints = map[string]*Constraint{constraint.Name: constraint} before, err := json.Marshal(table) if err != nil { t.Fatal(err) } index.AttachedPartitions = []IndexRef{{Schema: "public", Name: "ignored_child"}} + constraint.BackingIndex = &IndexRef{Schema: "public", Name: "unique_constraint"} table.AllPartitionChildren = []TableDependency{{Schema: "public", Name: "ignored_partition"}} after, err := json.Marshal(table) if err != nil { diff --git a/ir/inspector.go b/ir/inspector.go index 734963de..da590775 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -818,13 +818,11 @@ func (i *Inspector) buildIndexes(ctx context.Context, schema *IR, targetSchema s return err } - // Capture attachments before filtering so dump can detect missing children. - attachedPartitions := make(map[IndexRef][]IndexRef) - for _, row := range indexes { - if row.ParentIndex.Valid { - parent := IndexRef{Schema: row.ParentIndexSchema.String, Name: row.ParentIndex.String} - attachedPartitions[parent] = append(attachedPartitions[parent], IndexRef{Schema: row.Schemaname, Name: row.Indexname}) - } + // Read all attachment edges, including children owned by constraints and + // excluded objects. The managed-index query deliberately omits those rows. + attachedPartitions, err := i.buildIndexAttachments(ctx, schema, targetSchema) + if err != nil { + return err } for _, indexRow := range indexes { diff --git a/ir/ir.go b/ir/ir.go index 60aa37f9..ea3c8dc6 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -246,6 +246,9 @@ type Sequence struct { // Constraint represents a table constraint type Constraint struct { + // BackingIndex identifies the index created by this constraint. Inspection + // records it for attached indexes; it does not change constraint generation. + BackingIndex *IndexRef `json:"-"` Schema string `json:"schema"` Table string `json:"table"` Name string `json:"name"` diff --git a/ir/partition_index.go b/ir/partition_index.go new file mode 100644 index 00000000..70b8ff0c --- /dev/null +++ b/ir/partition_index.go @@ -0,0 +1,51 @@ +package ir + +import ( + "context" + "database/sql" +) + +// buildIndexAttachments reads catalog edges independently of managed-index +// filtering. Retained constraints identify the child indexes they will create. +func (i *Inspector) buildIndexAttachments(ctx context.Context, schema *IR, targetSchema string) (map[IndexRef][]IndexRef, error) { + rows, err := i.db.QueryContext(ctx, ` +SELECT pn.nspname, parent.relname, cn.nspname, child.relname, + t.relname, c.conname +FROM pg_inherits inh +JOIN pg_class parent ON parent.oid = inh.inhparent +JOIN pg_namespace pn ON pn.oid = parent.relnamespace +JOIN pg_class child ON child.oid = inh.inhrelid +JOIN pg_namespace cn ON cn.oid = child.relnamespace +JOIN pg_index idx ON idx.indexrelid = child.oid +JOIN pg_class t ON t.oid = idx.indrelid +LEFT JOIN pg_constraint c ON c.conindid = child.oid + AND c.conrelid = t.oid AND c.contype IN ('p', 'u', 'x') +WHERE parent.relkind = 'I' AND pn.nspname = $1 +ORDER BY pn.nspname, parent.relname, cn.nspname, child.relname`, targetSchema) + if err != nil { + return nil, err + } + defer rows.Close() + + attachments := make(map[IndexRef][]IndexRef) + for rows.Next() { + var parent, child IndexRef + var tableName string + var constraintName sql.NullString + if err := rows.Scan(&parent.Schema, &parent.Name, &child.Schema, &child.Name, &tableName, &constraintName); err != nil { + return nil, err + } + attachments[parent] = append(attachments[parent], child) + if namespace := schema.Schemas[child.Schema]; namespace != nil { + if table := namespace.Tables[tableName]; table != nil && constraintName.Valid { + if constraint := table.Constraints[constraintName.String]; constraint != nil { + constraint.BackingIndex = &child + } + } + } + } + if err := rows.Err(); err != nil { + return nil, err + } + return attachments, nil +} From 83da9e156549e862f1c78a967db77d63a3c2d238 Mon Sep 17 00:00:00 2001 From: Nikolay Kushin Date: Wed, 14 Oct 2026 10:02:20 +0200 Subject: [PATCH 6/6] fix(plan): distinguish partition index attachments from rebuilds --- internal/diff/attach_test.go | 63 +++++++++++ internal/diff/diff.go | 6 ++ internal/diff/partition_index.go | 2 +- internal/plan/partition_index_test.go | 144 ++++++++++++++++++++++++++ internal/plan/plan.go | 5 + 5 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 internal/diff/attach_test.go create mode 100644 internal/plan/partition_index_test.go diff --git a/internal/diff/attach_test.go b/internal/diff/attach_test.go new file mode 100644 index 00000000..72b530c6 --- /dev/null +++ b/internal/diff/attach_test.go @@ -0,0 +1,63 @@ +package diff + +import ( + "encoding/json" + "testing" + + "github.com/pgplex/pgschema/ir" +) + +func TestDiffOperationSerialization(t *testing.T) { + for value, name := range []string{"create", "alter", "drop", "recreate", "attach"} { + op := DiffOperation(value) + if op.String() != name { + t.Fatalf("operation %d = %q, want %q", value, op.String(), name) + } + encoded, err := json.Marshal(op) + if err != nil || string(encoded) != `"`+name+`"` { + t.Fatalf("marshal %s = %s, %v", name, encoded, err) + } + var restored DiffOperation + if err := json.Unmarshal(encoded, &restored); err != nil || restored != op { + t.Fatalf("unmarshal %s = %v, %v", name, restored, err) + } + } + for _, input := range []string{`"unknown"`, `4`, `true`} { + var op DiffOperation + if err := json.Unmarshal([]byte(input), &op); err == nil { + t.Errorf("accepted invalid operation %s", input) + } + } +} + +func TestPartitionIndexAttachmentMetadata(t *testing.T) { + schema := partitionIndexTestSchema() + parent := schema.Schemas["public"].Tables["events"].Indexes["parent_lookup"] + dumps, err := GenerateDump(schema, "public", false) + if err != nil { + t.Fatal(err) + } + for name, diffs := range map[string][]Diff{ + "dump": dumps, + "migration": GenerateMigration(ir.NewIR(), schema, "public"), + } { + t.Run(name, func(t *testing.T) { + attachments := 0 + for _, d := range diffs { + if d.Operation != DiffOperationAttach { + continue + } + attachments++ + if d.Type != DiffTypeTableIndex || d.Path != "public.events.parent_lookup" || d.Source != parent { + t.Fatalf("attachment ownership changed: %+v", d) + } + if len(d.Statements) != 1 || d.Statements[0].SQL != "ALTER INDEX parent_lookup ATTACH PARTITION custom_lookup;" || !d.Statements[0].CanRunInTransaction { + t.Fatalf("attachment SQL changed: %+v", d.Statements) + } + } + if attachments != 1 { + t.Fatalf("got %d attachments, want 1", attachments) + } + }) + } +} diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 1d198a21..4b53e1b2 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -199,6 +199,8 @@ const ( // for dependency handling. The object will be recreated, so this should be // counted as a modification in summaries, not a destruction. DiffOperationRecreate + // DiffOperationAttach indicates an explicit partition index attachment. + DiffOperationAttach ) // String returns the string representation of DiffOperation @@ -212,6 +214,8 @@ func (d DiffOperation) String() string { return "drop" case DiffOperationRecreate: return "recreate" + case DiffOperationAttach: + return "attach" default: return "unknown" } @@ -238,6 +242,8 @@ func (d *DiffOperation) UnmarshalJSON(data []byte) error { *d = DiffOperationDrop case "recreate": *d = DiffOperationRecreate + case "attach": + *d = DiffOperationAttach default: return fmt.Errorf("unknown diff operation: %s", s) } diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go index c51c5753..4a6e349b 100644 --- a/internal/diff/partition_index.go +++ b/internal/diff/partition_index.go @@ -273,7 +273,7 @@ func (p *partitionIndexCreationPlan) generateSQL(targetSchema string, collector qualifyEntityNameMode(index.Schema, index.Name, targetSchema, collector.qualifySchema), qualifyEntityNameMode(child.Schema, child.Name, targetSchema, collector.qualifySchema)) collector.collect(&diffContext{ - Type: DiffTypeTableIndex, Operation: DiffOperationAlter, + Type: DiffTypeTableIndex, Operation: DiffOperationAttach, Path: fmt.Sprintf("%s.%s.%s", index.Schema, index.Table, index.Name), Source: index, CanRunInTransaction: true, }, sql) diff --git a/internal/plan/partition_index_test.go b/internal/plan/partition_index_test.go new file mode 100644 index 00000000..557c8bcc --- /dev/null +++ b/internal/plan/partition_index_test.go @@ -0,0 +1,144 @@ +package plan + +import ( + "reflect" + "strings" + "testing" + + "github.com/pgplex/pgschema/internal/diff" + "github.com/pgplex/pgschema/ir" +) + +func TestPartitionIndexAttachments(t *testing.T) { + desired := parseSQL(t, ` +CREATE TABLE events (id integer) PARTITION BY RANGE (id); +CREATE TABLE events_early PARTITION OF events FOR VALUES FROM (0) TO (10) PARTITION BY RANGE (id); +CREATE TABLE events_leaf PARTITION OF events_early FOR VALUES FROM (0) TO (5); +CREATE TABLE events_later PARTITION OF events FOR VALUES FROM (10) TO (20); +CREATE INDEX leaf_lookup ON events_leaf (id); +CREATE INDEX later_lookup ON events_later (id); +CREATE INDEX early_lookup ON ONLY events_early (id); +ALTER INDEX early_lookup ATTACH PARTITION leaf_lookup; +CREATE INDEX parent_lookup ON ONLY events (id); +ALTER INDEX parent_lookup ATTACH PARTITION early_lookup; +ALTER INDEX parent_lookup ATTACH PARTITION later_lookup; +`) + diffs := diff.GenerateMigration(ir.NewIR(), desired, "public") + p := NewPlan(diffs, 18, nil) + if len(p.Groups) != 1 { + t.Fatalf("got %d groups, want one transaction", len(p.Groups)) + } + var canonical []Step + var attachments []string + for _, d := range diffs { + for _, stmt := range d.Statements { + canonical = append(canonical, Step{SQL: stmt.SQL, Type: d.Type.String(), Operation: d.Operation.String(), Path: d.Path}) + if d.Operation == diff.DiffOperationAttach { + attachments = append(attachments, stmt.SQL) + index, ok := d.Source.(*ir.Index) + if !ok || d.Path != index.Schema+"."+index.Table+"."+index.Name || d.Type != diff.DiffTypeTableIndex || !stmt.CanRunInTransaction { + t.Fatalf("invalid attachment metadata: %+v", d) + } + } + } + } + wantAttachments := []string{ + "ALTER INDEX early_lookup ATTACH PARTITION leaf_lookup;", + "ALTER INDEX parent_lookup ATTACH PARTITION early_lookup;", + "ALTER INDEX parent_lookup ATTACH PARTITION later_lookup;", + } + if !reflect.DeepEqual(attachments, wantAttachments) { + t.Fatalf("attachments = %v, want %v", attachments, wantAttachments) + } + if !reflect.DeepEqual(p.Groups[0].Steps, canonical) { + t.Fatalf("plan changed canonical statements or order: %+v", p.Groups) + } + human := p.HumanColored(false) + for _, name := range []string{"early_lookup", "parent_lookup"} { + if !strings.Contains(human, name+" (index attachment)") { + t.Errorf("missing attachment label for %s:\n%s", name, human) + } + } + if strings.Contains(human, "concurrent rebuild") { + t.Fatalf("attachment labeled as rebuild:\n%s", human) + } + summary := p.calculateSummaryFromSteps() + if summary.Total != 4 || summary.Add != 4 || summary.Change != 0 || summary.Destroy != 0 || summary.ByType["tables"].Add != 4 { + t.Fatalf("unexpected table counts: %+v", summary) + } + checkIndexPlanRoundTrips(t, p) +} + +func TestIndexReplacementRetainsConcurrentRewrite(t *testing.T) { + old := parseSQL(t, `CREATE TABLE users (id integer, name text); CREATE INDEX lookup ON users (id);`) + desired := parseSQL(t, `CREATE TABLE users (id integer, name text); CREATE INDEX lookup ON users (name);`) + p := NewPlan(diff.GenerateMigration(old, desired, "public"), 18, old) + human := p.HumanColored(false) + if !strings.Contains(human, "lookup (index - concurrent rebuild)") || strings.Contains(human, "index attachment") { + t.Fatalf("unexpected replacement label:\n%s", human) + } + sql := p.ToSQL(SQLFormatRaw) + if !strings.Contains(sql, "CREATE INDEX CONCURRENTLY IF NOT EXISTS lookup_pgschema_new ON users (name);") || + !strings.Contains(sql, "DROP INDEX IF EXISTS lookup;") || + !strings.Contains(sql, "ALTER INDEX lookup_pgschema_new RENAME TO lookup;") || len(p.Groups) != 3 { + t.Fatalf("replacement lost concurrent rewrite:\n%s", sql) + } + for _, group := range p.Groups { + for _, step := range group.Steps { + if step.Operation != "alter" { + t.Fatalf("replacement operation = %q, want alter", step.Operation) + } + } + } + checkIndexPlanRoundTrips(t, p) +} + +func checkIndexPlanRoundTrips(t *testing.T, p *Plan) { + t.Helper() + for _, debug := range []bool{false, true} { + name := "normal" + if debug { + name = "debug" + } + t.Run(name, func(t *testing.T) { + var encoded string + var err error + if debug { + encoded, err = p.ToJSONWithDebug(true) + } else { + encoded, err = p.ToJSON() + } + if err != nil { + t.Fatal(err) + } + restored, err := FromJSON([]byte(encoded)) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(restored.Groups, p.Groups) { + t.Fatal("JSON round trip changed groups, SQL, directives, or metadata") + } + for _, format := range []SQLFormat{SQLFormatRaw, SQLFormatHuman} { + if restored.ToSQL(format) != p.ToSQL(format) { + t.Errorf("JSON round trip changed %s SQL", format) + } + } + if !reflect.DeepEqual(restored.calculateSummaryFromSteps(), p.calculateSummaryFromSteps()) { + t.Fatal("JSON round trip changed summary counts") + } + if debug { + if len(restored.SourceDiffs) != len(p.SourceDiffs) { + t.Fatal("debug JSON lost source diffs") + } + for i, d := range restored.SourceDiffs { + original := p.SourceDiffs[i] + if d.Operation != original.Operation || d.Type != original.Type || d.Path != original.Path || !reflect.DeepEqual(d.Statements, original.Statements) { + t.Fatalf("debug JSON changed source diff %d", i) + } + } + } else if len(restored.SourceDiffs) != 0 { + t.Fatal("normal JSON unexpectedly includes source diffs") + } + }) + } +} diff --git a/internal/plan/plan.go b/internal/plan/plan.go index 096bc47c..400269a6 100644 --- a/internal/plan/plan.go +++ b/internal/plan/plan.go @@ -825,6 +825,11 @@ func (p *Plan) writeTableChanges(summary *strings.Builder, c *color.Color) { // Extract object name from source objectName := getObjectNameFromSource(subRes.source) + if subRes.subType == diff.DiffTypeTableIndex.String() && subRes.operation == diff.DiffOperationAttach.String() { + fmt.Fprintf(summary, " %s %s (index attachment)\n", c.PlanSymbol("change"), objectName) + continue + } + // Handle online index replacement display if subRes.subType == diff.DiffTypeTableIndex.String() && subRes.operation == diff.DiffOperationAlter.String() { subSymbol := c.PlanSymbol("change")