From 839bd60fc74a797f800269a07e203130b9d7e6d5 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 26 Oct 2026 20:08:05 -0700 Subject: [PATCH 1/4] fix: keep inherited constraint index names when dumping partitioned tables (#653) PARTITION OF copies a parent's inline PRIMARY KEY and UNIQUE constraints onto the new partition under generated names. A dump created partitions before the standalone indexes, so when a standalone index held such a name (for example events_leaf_pkey), the copy took it. The later CREATE INDEX IF NOT EXISTS was skipped and ALTER INDEX ... ATTACH PARTITION failed with SQLSTATE 42809, or a same-named index on another column was silently dropped. pgschema apply of its own dump failed the same way. Follow pg_dump instead. A PRIMARY KEY or UNIQUE constraint on a partitioned table with partitions is no longer written inline. After all tables exist the dump adds it with ALTER TABLE ONLY, adds each partition's copy under its catalog name the same way, and attaches the copies with ALTER INDEX ... ATTACH PARTITION, nested partitions included. Key columns carry NOT NULL explicitly. Foreign keys referencing the table or its partitions wait for these constraints. The inspector records the partition copies' names as inspection metadata, excluded from serialization and fingerprints; when a name is missing the constraint stays inline as before. Migration generation applies the same ordering to complete new hierarchies, which planPartitionIndexCreation already selects for explicit index attachment. Co-Authored-By: Claude Fable 5.1 --- cmd/dump/dump_integration_test.go | 7 + cmd/dump/partition_index_integration_test.go | 11 + docs/cli/dump.mdx | 9 + internal/diff/diff.go | 3 + internal/diff/partition_index.go | 135 +++++++++ internal/diff/table.go | 13 +- ir/index_test.go | 1 + ir/inspector.go | 26 ++ ir/ir.go | 7 + ir/queries/queries.sql | 25 ++ ir/queries/queries.sql.go | 65 +++++ .../diff.sql | 67 +++++ .../new.sql | 31 ++ .../old.sql | 1 + .../plan.json | 188 ++++++++++++ .../plan.sql | 67 +++++ .../plan.txt | 93 ++++++ .../issue_409_partitioned_fk/pgschema.sql | 25 +- .../pgschema.sql | 13 +- .../manifest.json | 10 + .../pgdump.sql | 272 ++++++++++++++++++ .../pgschema.sql | 123 ++++++++ .../raw.sql | 40 +++ testdata/dump/sakila/pgschema.sql | 25 +- 24 files changed, 1241 insertions(+), 16 deletions(-) create mode 100644 testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql create mode 100644 testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql create mode 100644 testdata/diff/create_table/issue_653_inherited_constraint_index_name/old.sql create mode 100644 testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json create mode 100644 testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql create mode 100644 testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt create mode 100644 testdata/dump/issue_653_inherited_constraint_index_name/manifest.json create mode 100644 testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql create mode 100644 testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql create mode 100644 testdata/dump/issue_653_inherited_constraint_index_name/raw.sql diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index 81690f634..940dbcc22 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -173,6 +173,13 @@ func TestDumpCommand_Issue472PartitionCloneTrigger(t *testing.T) { runExactMatchTest(t, "issue_472_partition_clone_trigger") } +func TestDumpCommand_Issue653InheritedConstraintIndexName(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + runExactMatchTest(t, "issue_653_inherited_constraint_index_name") +} + func TestDumpCommand_Issue421QuotedFKColumns(t *testing.T) { if testing.Short() { t.Skip("Skipping integration test in short mode") diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go index 72afb7bc5..01a23ed1d 100644 --- a/cmd/dump/partition_index_integration_test.go +++ b/cmd/dump/partition_index_integration_test.go @@ -78,6 +78,17 @@ 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'; +-- Inherited constraint indexes keep their catalog names even when a standalone +-- index already holds the generated name (issue #653). +CREATE TABLE keyed_events (bucket int NOT NULL, id int NOT NULL, note text, + PRIMARY KEY (bucket, id), CONSTRAINT keyed_events_note_key UNIQUE (bucket, id, note)) PARTITION BY RANGE (bucket); +CREATE INDEX keyed_events_early_pkey ON keyed_events (id); +CREATE TABLE keyed_events_early PARTITION OF keyed_events FOR VALUES FROM (0) TO (10); +CREATE TABLE keyed_events_late PARTITION OF keyed_events FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (bucket); +CREATE TABLE keyed_events_late_a PARTITION OF keyed_events_late FOR VALUES FROM (10) TO (15); +ALTER INDEX keyed_events_late_a_pkey RENAME TO keyed_late_a_pk; +CREATE INDEX keyed_events_late_a_pkey ON keyed_events_late_a (note); +CREATE TABLE keyed_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCES keyed_events (bucket, id)); ` for _, constraintType := range []string{"UNIQUE", "PRIMARY KEY"} { for _, multiFile := range []bool{false, true} { diff --git a/docs/cli/dump.mdx b/docs/cli/dump.mdx index e214d164d..c4442d4d0 100644 --- a/docs/cli/dump.mdx +++ b/docs/cli/dump.mdx @@ -263,6 +263,15 @@ leaves unattached indexes unattached. Multi-file dumps write parent indexes and their attachments under `indexes/`, such as `indexes/0001_events_lookup.sql`, after the table includes. +A `PRIMARY KEY` or `UNIQUE` constraint on a partitioned table with partitions is +not written inline. `PARTITION OF` would copy it onto each partition under a +generated name, which can take the name of a standalone index created later. +Instead, `pgschema` adds the constraint after the tables with +`ALTER TABLE ONLY ... ADD CONSTRAINT`, adds each partition's copy under its +original name the same way, and attaches the copies with +`ALTER INDEX ... ATTACH PARTITION`. Multi-file dumps write each constraint and +its copies under `constraints/`, such as `constraints/0001_events.sql`. + Each individual file (e.g., `tables/users.sql`) contains the specific object definition: ```sql -- diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 4b53e1b24..8a5a430c8 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -2153,6 +2153,9 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateSequenceComment(seq, targetSchema, DiffOperationCreate, collector) } + // Postponed PRIMARY KEY and UNIQUE constraints of partitioned tables come + // before the standalone partitioned indexes, which may attach to them. + d.partitionIndexes.generateDeferredConstraintSQL(targetSchema, collector) d.partitionIndexes.generateSQL(targetSchema, collector) // Add deferred foreign key constraints from ALL batches AFTER all tables are created diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go index 4a6e349b1..b44fe9d12 100644 --- a/internal/diff/partition_index.go +++ b/internal/diff/partition_index.go @@ -109,6 +109,16 @@ type partitionIndexCreationPlan struct { indexes map[ir.IndexRef]*ir.Index children map[ir.IndexRef]map[ir.IndexRef]bool uniqueTables map[string]bool + // constraints holds, per table key, the PRIMARY KEY and UNIQUE constraints + // added after every partition exists instead of inline with CREATE TABLE. + // PARTITION OF copies an inline constraint onto the new partition under a + // generated name, which can take the name of a standalone index created + // later (issue #653). Explicit ALTER TABLE ONLY ... ADD CONSTRAINT + // statements reproduce the catalog names instead. + constraints map[string]map[string]bool + // partitions lists the added partitions of each table key, in name order. + partitions map[string][]*ir.Table + tables map[string]*ir.Table } func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[string]*ir.Table, dump bool) *partitionIndexCreationPlan { @@ -116,6 +126,8 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin indexes: make(map[ir.IndexRef]*ir.Index), children: make(map[ir.IndexRef]map[ir.IndexRef]bool), uniqueTables: make(map[string]bool), + constraints: make(map[string]map[string]bool), + partitions: make(map[string][]*ir.Table), } added := make(map[string]*ir.Table) available := availablePartitionIndexes(addedTables) @@ -128,6 +140,26 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin } } } + plan.tables = added + for _, table := range addedTables { + if table.PartitionOf == "" { + continue + } + parentSchema := table.PartitionOfSchema + if parentSchema == "" { + parentSchema = table.Schema + } + parentKey := parentSchema + "." + table.PartitionOf + plan.partitions[parentKey] = append(plan.partitions[parentKey], table) + } + for _, partitions := range plan.partitions { + sort.Slice(partitions, func(i, j int) bool { + if partitions[i].Schema != partitions[j].Schema { + return partitions[i].Schema < partitions[j].Schema + } + return partitions[i].Name < partitions[j].Name + }) + } roots := make(map[string]string) blocked := make(map[string]bool) @@ -174,6 +206,27 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin } } } + // A constraint is postponed only when every partition copy's catalog + // name is known; otherwise inline creation remains the fallback. + if len(plan.partitions[key]) == 0 { + continue + } + for _, name := range sortedKeys(table.Constraints) { + constraint := table.Constraints[name] + if constraint.Type != ir.ConstraintTypePrimaryKey && constraint.Type != ir.ConstraintTypeUnique { + continue + } + if !plan.inheritedNamesKnown(table, name) { + continue + } + if plan.constraints[key] == nil { + plan.constraints[key] = make(map[string]bool) + } + plan.constraints[key][name] = true + // Foreign keys referencing the table or any of its partitions wait + // for the postponed constraints. + plan.markUniqueTables(table) + } } for parent, index := range plan.indexes { plan.children[parent] = make(map[ir.IndexRef]bool) @@ -217,6 +270,88 @@ func (p *partitionIndexCreationPlan) contains(index *ir.Index) bool { return p != nil && p.indexes[ir.IndexRef{Schema: index.Schema, Name: index.Name}] != nil } +// inheritedNamesKnown reports whether every partition below table, at any +// depth, records the catalog name of its copy of the named constraint. +func (p *partitionIndexCreationPlan) inheritedNamesKnown(table *ir.Table, constraintName string) bool { + for _, child := range p.partitions[table.Schema+"."+table.Name] { + childName := child.InheritedConstraints[constraintName] + if childName == "" || !p.inheritedNamesKnown(child, childName) { + return false + } + } + return true +} + +func (p *partitionIndexCreationPlan) markUniqueTables(table *ir.Table) { + key := table.Schema + "." + table.Name + p.uniqueTables[key] = true + for _, child := range p.partitions[key] { + p.markUniqueTables(child) + } +} + +// defersConstraint reports whether the constraint is added after the +// partitions exist rather than inline with its table. +func (p *partitionIndexCreationPlan) defersConstraint(table *ir.Table, constraint *ir.Constraint) bool { + if p == nil || table == nil || constraint == nil { + return false + } + return p.constraints[table.Schema+"."+table.Name][constraint.Name] +} + +// generateDeferredConstraintSQL adds the postponed PRIMARY KEY and UNIQUE +// constraints once every partition exists. ONLY keeps PostgreSQL from creating +// partition copies under generated names; each copy is added under its catalog +// name and attached explicitly. One step carries the whole family so the +// statements stay together in plans and multi-file dumps. +func (p *partitionIndexCreationPlan) generateDeferredConstraintSQL(targetSchema string, collector *diffCollector) { + if p == nil { + return + } + keys := make([]string, 0, len(p.constraints)) + for key := range p.constraints { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + table := p.tables[key] + for _, name := range sortedKeys(p.constraints[key]) { + constraint := table.Constraints[name] + var statements []SQLStatement + p.appendConstraintFamily(&statements, table, constraint, targetSchema, collector.qualifySchema) + collector.collectStatements(&diffContext{ + Type: DiffTypeTableConstraint, Operation: DiffOperationCreate, + Path: fmt.Sprintf("%s.%s.%s", table.Schema, table.Name, constraint.Name), + Source: constraint, CanRunInTransaction: true, + }, statements) + } + } +} + +// appendConstraintFamily adds the constraint to its table with ONLY, then each +// partition's copy under its recorded name, attaching every copy to the +// constraint's index. Nested partitions attach to their immediate parent. +func (p *partitionIndexCreationPlan) appendConstraintFamily(statements *[]SQLStatement, table *ir.Table, constraint *ir.Constraint, targetSchema string, qualifySchema bool) { + tableName := qualifyEntityNameMode(table.Schema, table.Name, targetSchema, qualifySchema) + *statements = append(*statements, SQLStatement{ + SQL: fmt.Sprintf("ALTER TABLE ONLY %s ADD %s;", tableName, generateConstraintSQL(constraint, targetSchema, qualifySchema)), + CanRunInTransaction: true, + }) + for _, child := range p.partitions[table.Schema+"."+table.Name] { + childConstraint := *constraint + childConstraint.Schema = child.Schema + childConstraint.Table = child.Name + childConstraint.Name = child.InheritedConstraints[constraint.Name] + p.appendConstraintFamily(statements, child, &childConstraint, targetSchema, qualifySchema) + *statements = append(*statements, SQLStatement{ + SQL: fmt.Sprintf("ALTER INDEX %s ATTACH PARTITION %s;", + qualifyEntityNameMode(table.Schema, constraint.Name, targetSchema, qualifySchema), + qualifyEntityNameMode(child.Schema, childConstraint.Name, targetSchema, qualifySchema)), + CanRunInTransaction: true, + }) + } +} + func (p *partitionIndexCreationPlan) defersForeignKey(table *ir.Table, constraint *ir.Constraint) bool { if p == nil || constraint == nil || constraint.Type != ir.ConstraintTypeForeignKey { return false diff --git a/internal/diff/table.go b/internal/diff/table.go index c9fc2e6d5..e36ad7ddf 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -987,7 +987,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, var deferred []*deferredConstraint currentKey := fmt.Sprintf("%s.%s", table.Schema, table.Name) for _, constraint := range inlineConstraints { - if suppressedInlineFKs[constraintPathKey(constraint)] { + if suppressedInlineFKs[constraintPathKey(constraint)] || partitionIndexes.defersConstraint(table, constraint) { continue } if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) || @@ -1031,7 +1031,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, for _, column := range table.Columns { // Build column definition with SERIAL detection var builder strings.Builder - writeColumnDefinitionToBuilder(&builder, table, column, targetSchema, qualifySchema) + writeColumnDefinitionToBuilder(&builder, table, column, targetSchema, qualifySchema, partitionIndexes) columnParts = append(columnParts, fmt.Sprintf(" %s", builder.String())) } @@ -1052,7 +1052,7 @@ func generateTableSQL(table *ir.Table, targetSchema string, qualifySchema bool, for _, constraint := range inlineConstraints { // FKs bound to a unique/PK constraint being replaced by this migration // are created after the replacement instead (issue #439) - if suppressedInlineFKs[constraintPathKey(constraint)] { + if suppressedInlineFKs[constraintPathKey(constraint)] || partitionIndexes.defersConstraint(table, constraint) { continue } if shouldDeferConstraint(table, constraint, currentKey, createdTables, existingTables) || @@ -1969,7 +1969,10 @@ func ensureCheckClauseParens(s string) string { // writeColumnDefinitionToBuilder builds column definitions with SERIAL detection and proper formatting // This is moved from ir/table.go to consolidate SQL generation in the diff module -func writeColumnDefinitionToBuilder(builder *strings.Builder, table *ir.Table, column *ir.Column, targetSchema string, qualifySchema bool) { +// A PRIMARY KEY postponed until the partitions exist (see +// partitionIndexCreationPlan) does not mark its columns NOT NULL inline, so +// the column definitions carry NOT NULL themselves, as pg_dump writes them. +func writeColumnDefinitionToBuilder(builder *strings.Builder, table *ir.Table, column *ir.Column, targetSchema string, qualifySchema bool, partitionIndexes *partitionIndexCreationPlan) { builder.WriteString(ir.QuoteIdentifier(column.Name)) builder.WriteString(" ") @@ -1985,7 +1988,7 @@ func writeColumnDefinitionToBuilder(builder *strings.Builder, table *ir.Table, c // Check if column is part of any primary key constraint for NOT NULL handling isPartOfAnyPK := false for _, constraint := range table.Constraints { - if constraint.Type == ir.ConstraintTypePrimaryKey { + if constraint.Type == ir.ConstraintTypePrimaryKey && !partitionIndexes.defersConstraint(table, constraint) { for _, col := range constraint.Columns { if col.Name == column.Name { isPartOfAnyPK = true diff --git a/ir/index_test.go b/ir/index_test.go index 2cf1cc012..fecd59c26 100644 --- a/ir/index_test.go +++ b/ir/index_test.go @@ -17,6 +17,7 @@ func TestPartitionInspectionMetadataNotSerialized(t *testing.T) { index.AttachedPartitions = []IndexRef{{Schema: "public", Name: "ignored_child"}} constraint.BackingIndex = &IndexRef{Schema: "public", Name: "unique_constraint"} table.AllPartitionChildren = []TableDependency{{Schema: "public", Name: "ignored_partition"}} + table.InheritedConstraints = map[string]string{"events_pkey": "events_early_pkey1"} after, err := json.Marshal(table) if err != nil { t.Fatal(err) diff --git a/ir/inspector.go b/ir/inspector.go index 7ccd0f0d1..77a5519f8 100644 --- a/ir/inspector.go +++ b/ir/inspector.go @@ -734,6 +734,9 @@ func (i *Inspector) buildConstraints(ctx context.Context, schema *IR, targetSche // Record which tables are partitions of which parents i.populatePartitionChildMetadata(ctx, schema, targetSchema) + if err := i.buildInheritedConstraints(ctx, schema, targetSchema); err != nil { + return err + } // Add constraints to tables for key, constraint := range constraintGroups { @@ -762,6 +765,29 @@ func (i *Inspector) buildConstraints(ctx context.Context, schema *IR, targetSche return nil } +// buildInheritedConstraints records the catalog names of the PRIMARY KEY and +// UNIQUE constraints that partitions inherited from their parents. They are not +// managed constraints (see GetConstraintsForSchema), but creation planning needs +// their names to recreate them explicitly (issue #653). +func (i *Inspector) buildInheritedConstraints(ctx context.Context, schema *IR, targetSchema string) error { + rows, err := i.queries.GetInheritedPartitionConstraintsForSchema(ctx, sql.NullString{String: targetSchema, Valid: true}) + if err != nil { + return err + } + dbSchema := schema.getOrCreateSchema(targetSchema) + for _, row := range rows { + table, exists := dbSchema.Tables[row.TableName] + if !exists { + continue + } + if table.InheritedConstraints == nil { + table.InheritedConstraints = make(map[string]string) + } + table.InheritedConstraints[row.ParentConstraintName] = row.ConstraintName + } + return nil +} + // requiresPositionSorting returns true if the constraint type requires columns to be sorted by position func requiresPositionSorting(constraintType ConstraintType) bool { switch constraintType { diff --git a/ir/ir.go b/ir/ir.go index ea3c8dc6e..91bda3010 100644 --- a/ir/ir.go +++ b/ir/ir.go @@ -83,6 +83,13 @@ type Table struct { // AllPartitionChildren records catalog children, including ignored or external // tables. Creation planning uses this metadata to identify complete hierarchies. AllPartitionChildren []TableDependency `json:"-"` + // InheritedConstraints maps a parent constraint name to the name PostgreSQL + // gave this partition's copy of it, for PRIMARY KEY and UNIQUE constraints. + // The copies are not managed constraints; creation planning uses the names + // to recreate them explicitly so a generated copy cannot take a standalone + // index's name (issue #653). Inspection metadata, excluded from + // serialization and fingerprints. + InheritedConstraints map[string]string `json:"-"` } // Column represents a table column diff --git a/ir/queries/queries.sql b/ir/queries/queries.sql index 7b97e7c3c..80234eb6a 100644 --- a/ir/queries/queries.sql +++ b/ir/queries/queries.sql @@ -1466,6 +1466,31 @@ WHERE n.nspname = $1 ) ORDER BY n.nspname, cl.relname, c.contype, c.conname, a.attnum; +-- GetInheritedPartitionConstraintsForSchema lists the PRIMARY KEY and UNIQUE +-- constraints PostgreSQL copied onto partitions from their parent's constraints +-- (conparentid != 0), with the parent constraint each one derives from. +-- GetConstraintsForSchema skips these copies because PARTITION OF recreates +-- them, but their catalog names are needed to recreate them explicitly when a +-- generated name would otherwise take a standalone index's name (issue #653). +-- name: GetInheritedPartitionConstraintsForSchema :many +SELECT + n.nspname AS table_schema, + cl.relname AS table_name, + c.conname AS constraint_name, + pn.nspname AS parent_schema, + pcl.relname AS parent_table, + pc.conname AS parent_constraint_name +FROM pg_constraint c +JOIN pg_class cl ON c.conrelid = cl.oid +JOIN pg_namespace n ON cl.relnamespace = n.oid +JOIN pg_constraint pc ON pc.oid = c.conparentid +JOIN pg_class pcl ON pc.conrelid = pcl.oid +JOIN pg_namespace pn ON pcl.relnamespace = pn.oid +WHERE n.nspname = $1 + AND c.contype IN ('p', 'u') + AND c.conparentid <> 0 +ORDER BY n.nspname, cl.relname, c.conname; + -- GetSequencesForSchema retrieves all sequences for a specific schema -- name: GetSequencesForSchema :many SELECT diff --git a/ir/queries/queries.sql.go b/ir/queries/queries.sql.go index 48d076d5a..acf14b0ce 100644 --- a/ir/queries/queries.sql.go +++ b/ir/queries/queries.sql.go @@ -2414,6 +2414,71 @@ func (q *Queries) GetIndexesForSchema(ctx context.Context, dollar_1 sql.NullStri return items, nil } +const getInheritedPartitionConstraintsForSchema = `-- name: GetInheritedPartitionConstraintsForSchema :many +SELECT + n.nspname AS table_schema, + cl.relname AS table_name, + c.conname AS constraint_name, + pn.nspname AS parent_schema, + pcl.relname AS parent_table, + pc.conname AS parent_constraint_name +FROM pg_constraint c +JOIN pg_class cl ON c.conrelid = cl.oid +JOIN pg_namespace n ON cl.relnamespace = n.oid +JOIN pg_constraint pc ON pc.oid = c.conparentid +JOIN pg_class pcl ON pc.conrelid = pcl.oid +JOIN pg_namespace pn ON pcl.relnamespace = pn.oid +WHERE n.nspname = $1 + AND c.contype IN ('p', 'u') + AND c.conparentid <> 0 +ORDER BY n.nspname, cl.relname, c.conname +` + +type GetInheritedPartitionConstraintsForSchemaRow struct { + TableSchema string `db:"table_schema" json:"table_schema"` + TableName string `db:"table_name" json:"table_name"` + ConstraintName string `db:"constraint_name" json:"constraint_name"` + ParentSchema string `db:"parent_schema" json:"parent_schema"` + ParentTable string `db:"parent_table" json:"parent_table"` + ParentConstraintName string `db:"parent_constraint_name" json:"parent_constraint_name"` +} + +// GetInheritedPartitionConstraintsForSchema lists the PRIMARY KEY and UNIQUE +// constraints PostgreSQL copied onto partitions from their parent's constraints +// (conparentid != 0), with the parent constraint each one derives from. +// GetConstraintsForSchema skips these copies because PARTITION OF recreates +// them, but their catalog names are needed to recreate them explicitly when a +// generated name would otherwise take a standalone index's name (issue #653). +func (q *Queries) GetInheritedPartitionConstraintsForSchema(ctx context.Context, dollar_1 sql.NullString) ([]GetInheritedPartitionConstraintsForSchemaRow, error) { + rows, err := q.db.QueryContext(ctx, getInheritedPartitionConstraintsForSchema, dollar_1) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetInheritedPartitionConstraintsForSchemaRow + for rows.Next() { + var i GetInheritedPartitionConstraintsForSchemaRow + if err := rows.Scan( + &i.TableSchema, + &i.TableName, + &i.ConstraintName, + &i.ParentSchema, + &i.ParentTable, + &i.ParentConstraintName, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getPartitionChildren = `-- name: GetPartitionChildren :many SELECT pn.nspname AS parent_schema, diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql new file mode 100644 index 000000000..d7547f1e6 --- /dev/null +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql @@ -0,0 +1,67 @@ +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + region text NOT NULL +) PARTITION BY RANGE (id); + +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +CREATE TABLE IF NOT EXISTS events_archive PARTITION OF events FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id); + +CREATE TABLE IF NOT EXISTS events_archive_old PARTITION OF events_archive FOR VALUES FROM (10) TO (15); + +CREATE INDEX IF NOT EXISTS events_archive_old_id_idx ON events_archive_old (id); + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) TO (10); + +CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); + +CREATE TABLE IF NOT EXISTS leaf_notes ( + event_id integer NOT NULL +); + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_pkey PRIMARY KEY (id); + +ALTER INDEX events_archive_pkey ATTACH PARTITION events_archive_old_pkey; + +ALTER INDEX events_pkey ATTACH PARTITION events_archive_pkey; + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id); + +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey1; + +ALTER TABLE ONLY events ADD CONSTRAINT events_region_id_key UNIQUE (region, id); + +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_region_id_key UNIQUE (region, id); + +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_region_id_key UNIQUE (region, id); + +ALTER INDEX events_archive_region_id_key ATTACH PARTITION events_archive_old_region_id_key; + +ALTER INDEX events_region_id_key ATTACH PARTITION events_archive_region_id_key; + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id); + +ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; + +CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); + +ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; + +CREATE INDEX IF NOT EXISTS events_leaf_pkey ON ONLY events (id); + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_archive_id_idx; + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_leaf_id_idx; + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + +ALTER TABLE leaf_notes +ADD CONSTRAINT leaf_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events_leaf (id); diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql new file mode 100644 index 000000000..4eb952247 --- /dev/null +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql @@ -0,0 +1,31 @@ +-- A standalone index on the parent holds the name PostgreSQL would generate +-- for the partition's primary key index, so the partition's copy becomes +-- events_leaf_pkey1 (issue #653). Creating the partition with an inline +-- PRIMARY KEY would hand that generated name to the copy and block the +-- standalone index. Constraints are added after the partitions instead, +-- each copy under its catalog name. +CREATE TABLE public.events ( + id integer PRIMARY KEY, + region text NOT NULL, + CONSTRAINT events_region_id_key UNIQUE (region, id) +) PARTITION BY RANGE (id); + +CREATE INDEX events_leaf_pkey ON public.events (id); + +CREATE TABLE public.events_leaf PARTITION OF public.events FOR VALUES FROM (0) TO (10); + +CREATE TABLE public.events_archive PARTITION OF public.events + FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id); + +CREATE TABLE public.events_archive_old PARTITION OF public.events_archive FOR VALUES FROM (10) TO (15); + +-- The foreign key waits for the partitioned primary key to be complete. +CREATE TABLE public.event_notes ( + event_id integer NOT NULL REFERENCES public.events (id), + note text +); + +-- So does a foreign key that targets a partition's copy of the key. +CREATE TABLE public.leaf_notes ( + event_id integer NOT NULL REFERENCES public.events_leaf (id) +); diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/old.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/old.sql new file mode 100644 index 000000000..03d7e9aef --- /dev/null +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/old.sql @@ -0,0 +1 @@ +-- Empty schema diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json new file mode 100644 index 000000000..a269412be --- /dev/null +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json @@ -0,0 +1,188 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.13.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "965b1131737c955e24c7f827c55bd78e4cb49a75adfd04229e0ba297376f5085" + }, + "groups": [ + { + "steps": [ + { + "sql": "CREATE TABLE IF NOT EXISTS events (\n id integer NOT NULL,\n region text NOT NULL\n) PARTITION BY RANGE (id);", + "type": "table", + "operation": "create", + "path": "public.events" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS event_notes (\n event_id integer NOT NULL,\n note text\n);", + "type": "table", + "operation": "create", + "path": "public.event_notes" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_archive PARTITION OF events FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id);", + "type": "table", + "operation": "create", + "path": "public.events_archive" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_archive_old PARTITION OF events_archive FOR VALUES FROM (10) TO (15);", + "type": "table", + "operation": "create", + "path": "public.events_archive_old" + }, + { + "sql": "CREATE INDEX IF NOT EXISTS events_archive_old_id_idx ON events_archive_old (id);", + "type": "table.index", + "operation": "create", + "path": "public.events_archive_old.events_archive_old_id_idx" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) TO (10);", + "type": "table", + "operation": "create", + "path": "public.events_leaf" + }, + { + "sql": "CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id);", + "type": "table.index", + "operation": "create", + "path": "public.events_leaf.events_leaf_id_idx" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS leaf_notes (\n event_id integer NOT NULL\n);", + "type": "table", + "operation": "create", + "path": "public.leaf_notes" + }, + { + "sql": "ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_pkey PRIMARY KEY (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER INDEX events_archive_pkey ATTACH PARTITION events_archive_old_pkey;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER INDEX events_pkey ATTACH PARTITION events_archive_pkey;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey1;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER TABLE ONLY events ADD CONSTRAINT events_region_id_key UNIQUE (region, id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_region_id_key UNIQUE (region, id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_region_id_key UNIQUE (region, id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "ALTER INDEX events_archive_region_id_key ATTACH PARTITION events_archive_old_region_id_key;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "ALTER INDEX events_region_id_key ATTACH PARTITION events_archive_region_id_key;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_region_id_key" + }, + { + "sql": "CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id);", + "type": "table.index", + "operation": "create", + "path": "public.events_archive.events_archive_id_idx" + }, + { + "sql": "ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx;", + "type": "table.index", + "operation": "attach", + "path": "public.events_archive.events_archive_id_idx" + }, + { + "sql": "CREATE INDEX IF NOT EXISTS events_leaf_pkey ON ONLY events (id);", + "type": "table.index", + "operation": "create", + "path": "public.events.events_leaf_pkey" + }, + { + "sql": "ALTER INDEX events_leaf_pkey ATTACH PARTITION events_archive_id_idx;", + "type": "table.index", + "operation": "attach", + "path": "public.events.events_leaf_pkey" + }, + { + "sql": "ALTER INDEX events_leaf_pkey ATTACH PARTITION events_leaf_id_idx;", + "type": "table.index", + "operation": "attach", + "path": "public.events.events_leaf_pkey" + }, + { + "sql": "ALTER TABLE event_notes\nADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.event_notes.event_notes_event_id_fkey" + }, + { + "sql": "ALTER TABLE leaf_notes\nADD CONSTRAINT leaf_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events_leaf (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.leaf_notes.leaf_notes_event_id_fkey" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql new file mode 100644 index 000000000..d7547f1e6 --- /dev/null +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql @@ -0,0 +1,67 @@ +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + region text NOT NULL +) PARTITION BY RANGE (id); + +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +CREATE TABLE IF NOT EXISTS events_archive PARTITION OF events FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id); + +CREATE TABLE IF NOT EXISTS events_archive_old PARTITION OF events_archive FOR VALUES FROM (10) TO (15); + +CREATE INDEX IF NOT EXISTS events_archive_old_id_idx ON events_archive_old (id); + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) TO (10); + +CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); + +CREATE TABLE IF NOT EXISTS leaf_notes ( + event_id integer NOT NULL +); + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_pkey PRIMARY KEY (id); + +ALTER INDEX events_archive_pkey ATTACH PARTITION events_archive_old_pkey; + +ALTER INDEX events_pkey ATTACH PARTITION events_archive_pkey; + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id); + +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey1; + +ALTER TABLE ONLY events ADD CONSTRAINT events_region_id_key UNIQUE (region, id); + +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_region_id_key UNIQUE (region, id); + +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_region_id_key UNIQUE (region, id); + +ALTER INDEX events_archive_region_id_key ATTACH PARTITION events_archive_old_region_id_key; + +ALTER INDEX events_region_id_key ATTACH PARTITION events_archive_region_id_key; + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id); + +ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; + +CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); + +ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; + +CREATE INDEX IF NOT EXISTS events_leaf_pkey ON ONLY events (id); + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_archive_id_idx; + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_leaf_id_idx; + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + +ALTER TABLE leaf_notes +ADD CONSTRAINT leaf_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events_leaf (id); diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt new file mode 100644 index 000000000..0a125c0ea --- /dev/null +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt @@ -0,0 +1,93 @@ +Plan: 6 to add. + +Summary by type: + tables: 6 to add + +Tables: + + event_notes + + event_notes_event_id_fkey (constraint) + + events + + events_pkey (constraint) + + events_region_id_key (constraint) + + events_leaf_pkey (index) + ~ events_leaf_pkey (index attachment) + + events_archive + + events_archive_id_idx (index) + ~ events_archive_id_idx (index attachment) + + events_archive_old + + events_archive_old_id_idx (index) + + events_leaf + + events_leaf_id_idx (index) + + leaf_notes + + leaf_notes_event_id_fkey (constraint) + +DDL to be executed: +-------------------------------------------------- + +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + region text NOT NULL +) PARTITION BY RANGE (id); + +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +CREATE TABLE IF NOT EXISTS events_archive PARTITION OF events FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id); + +CREATE TABLE IF NOT EXISTS events_archive_old PARTITION OF events_archive FOR VALUES FROM (10) TO (15); + +CREATE INDEX IF NOT EXISTS events_archive_old_id_idx ON events_archive_old (id); + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) TO (10); + +CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); + +CREATE TABLE IF NOT EXISTS leaf_notes ( + event_id integer NOT NULL +); + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_pkey PRIMARY KEY (id); + +ALTER INDEX events_archive_pkey ATTACH PARTITION events_archive_old_pkey; + +ALTER INDEX events_pkey ATTACH PARTITION events_archive_pkey; + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id); + +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey1; + +ALTER TABLE ONLY events ADD CONSTRAINT events_region_id_key UNIQUE (region, id); + +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_region_id_key UNIQUE (region, id); + +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_region_id_key UNIQUE (region, id); + +ALTER INDEX events_archive_region_id_key ATTACH PARTITION events_archive_old_region_id_key; + +ALTER INDEX events_region_id_key ATTACH PARTITION events_archive_region_id_key; + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id); + +ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; + +CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); + +ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; + +CREATE INDEX IF NOT EXISTS events_leaf_pkey ON ONLY events (id); + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_archive_id_idx; + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_leaf_id_idx; + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + +ALTER TABLE leaf_notes +ADD CONSTRAINT leaf_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events_leaf (id); diff --git a/testdata/dump/issue_409_partitioned_fk/pgschema.sql b/testdata/dump/issue_409_partitioned_fk/pgschema.sql index 51f3d45d2..b0fbb46e7 100644 --- a/testdata/dump/issue_409_partitioned_fk/pgschema.sql +++ b/testdata/dump/issue_409_partitioned_fk/pgschema.sql @@ -11,9 +11,8 @@ -- CREATE TABLE IF NOT EXISTS session ( - id bigint, - started_at timestamptz, - CONSTRAINT session_pkey PRIMARY KEY (id, started_at) + id bigint NOT NULL, + started_at timestamptz NOT NULL ) PARTITION BY RANGE (started_at); -- @@ -22,8 +21,7 @@ CREATE TABLE IF NOT EXISTS session ( CREATE TABLE IF NOT EXISTS event ( session_id bigint NOT NULL, - session_started_at timestamptz NOT NULL, - CONSTRAINT event_session_id_session_started_at_fkey FOREIGN KEY (session_id, session_started_at) REFERENCES session (id, started_at) + session_started_at timestamptz NOT NULL ); -- @@ -38,3 +36,20 @@ CREATE TABLE IF NOT EXISTS session_2026_01 PARTITION OF session FOR VALUES FROM CREATE TABLE IF NOT EXISTS session_2026_02 PARTITION OF session FOR VALUES FROM ('2026-02-01 00:00:00+00') TO ('2026-03-01 00:00:00+00'); +-- +-- Name: session_pkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY session ADD CONSTRAINT session_pkey PRIMARY KEY (id, started_at); +ALTER TABLE ONLY session_2026_01 ADD CONSTRAINT session_2026_01_pkey PRIMARY KEY (id, started_at); +ALTER INDEX session_pkey ATTACH PARTITION session_2026_01_pkey; +ALTER TABLE ONLY session_2026_02 ADD CONSTRAINT session_2026_02_pkey PRIMARY KEY (id, started_at); +ALTER INDEX session_pkey ATTACH PARTITION session_2026_02_pkey; + +-- +-- Name: event_session_id_session_started_at_fkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE event +ADD CONSTRAINT event_session_id_session_started_at_fkey FOREIGN KEY (session_id, session_started_at) REFERENCES session (id, started_at); + diff --git a/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql b/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql index 29eb85921..c8acebb24 100644 --- a/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql +++ b/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql @@ -11,10 +11,9 @@ -- CREATE TABLE IF NOT EXISTS ledger ( - id uuid, + id uuid NOT NULL, amount bigint NOT NULL, - ts timestamptz, - CONSTRAINT ledger_pkey PRIMARY KEY (ts, id) + ts timestamptz NOT NULL ) PARTITION BY RANGE (ts); -- @@ -34,6 +33,14 @@ VOLATILE AS $$ BEGIN RETURN NEW; END $$; +-- +-- Name: ledger_pkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY ledger ADD CONSTRAINT ledger_pkey PRIMARY KEY (ts, id); +ALTER TABLE ONLY ledger_2026_06 ADD CONSTRAINT ledger_2026_06_pkey PRIMARY KEY (ts, id); +ALTER INDEX ledger_pkey ATTACH PARTITION ledger_2026_06_pkey; + -- -- Name: trg_rollup; Type: TRIGGER; Schema: -; Owner: - -- diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/manifest.json b/testdata/dump/issue_653_inherited_constraint_index_name/manifest.json new file mode 100644 index 000000000..984da9808 --- /dev/null +++ b/testdata/dump/issue_653_inherited_constraint_index_name/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "issue_653_inherited_constraint_index_name", + "description": "Dump replay failed or silently dropped an index when a partition's inherited PRIMARY KEY index name collided with a standalone index", + "source": "https://github.com/pgplex/pgschema/issues/653", + "notes": [ + "PARTITION OF copies the parent's PRIMARY KEY and UNIQUE constraints onto the partition under generated names; with a standalone index already named events_leaf_pkey, the copy is named events_leaf_pkey1.", + "Creating the partition with the constraint inline, before the standalone index, gave the generated name to the copy. The later CREATE INDEX IF NOT EXISTS events_leaf_pkey was skipped and ALTER INDEX ... ATTACH PARTITION failed with SQLSTATE 42809.", + "The dump now follows pg_dump: PRIMARY KEY and UNIQUE constraints of a partitioned table with partitions are added after the tables with ALTER TABLE ONLY, each partition copy under its catalog name, followed by ALTER INDEX ... ATTACH PARTITION." + ] +} diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql b/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql new file mode 100644 index 000000000..da1d7912b --- /dev/null +++ b/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql @@ -0,0 +1,272 @@ +-- +-- PostgreSQL database dump +-- + + +-- Dumped from database version 17.6 (Homebrew) +-- Dumped by pg_dump version 17.6 (Homebrew) + +SET statement_timeout = 0; +SET lock_timeout = 0; +SET idle_in_transaction_session_timeout = 0; +-- SET transaction_timeout = 0; +SET client_encoding = 'SQL_ASCII'; +SET standard_conforming_strings = on; +SELECT pg_catalog.set_config('search_path', '', false); +SET check_function_bodies = false; +SET xmloption = content; +SET client_min_messages = warning; +SET row_security = off; + +SET default_tablespace = ''; + +SET default_table_access_method = heap; + +-- +-- Name: event_notes; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.event_notes ( + event_id integer NOT NULL, + note text +); + + +-- +-- Name: events; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.events ( + id integer NOT NULL, + region text NOT NULL +) +PARTITION BY RANGE (id); + + +-- +-- Name: events_archive; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.events_archive ( + id integer NOT NULL, + region text NOT NULL +) +PARTITION BY RANGE (id); + + +-- +-- Name: events_archive_old; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.events_archive_old ( + id integer NOT NULL, + region text NOT NULL +); + + +-- +-- Name: events_leaf; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.events_leaf ( + id integer NOT NULL, + region text NOT NULL +); + + +-- +-- Name: events_archive; Type: TABLE ATTACH; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events ATTACH PARTITION public.events_archive FOR VALUES FROM (10) TO (20); + + +-- +-- Name: events_archive_old; Type: TABLE ATTACH; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_archive ATTACH PARTITION public.events_archive_old FOR VALUES FROM (10) TO (15); + + +-- +-- Name: events_leaf; Type: TABLE ATTACH; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events ATTACH PARTITION public.events_leaf FOR VALUES FROM (0) TO (10); + + +-- +-- Name: events events_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events + ADD CONSTRAINT events_pkey PRIMARY KEY (id); + + +-- +-- Name: events_archive events_archive_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_archive + ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); + + +-- +-- Name: events_archive_old events_archive_old_pk; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_archive_old + ADD CONSTRAINT events_archive_old_pk PRIMARY KEY (id); + + +-- +-- Name: events events_region_id_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events + ADD CONSTRAINT events_region_id_key UNIQUE (region, id); + + +-- +-- Name: events_archive events_archive_region_id_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_archive + ADD CONSTRAINT events_archive_region_id_key UNIQUE (region, id); + + +-- +-- Name: events_archive_old events_archive_old_region_id_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_archive_old + ADD CONSTRAINT events_archive_old_region_id_key UNIQUE (region, id); + + +-- +-- Name: events_leaf events_leaf_pkey1; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_leaf + ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id); + + +-- +-- Name: events_leaf events_leaf_region_id_key; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.events_leaf + ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id); + + +-- +-- Name: events_leaf_pkey; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX events_leaf_pkey ON ONLY public.events USING btree (id); + + +-- +-- Name: events_archive_id_idx; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX events_archive_id_idx ON ONLY public.events_archive USING btree (id); + + +-- +-- Name: events_archive_old_id_idx; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX events_archive_old_id_idx ON public.events_archive_old USING btree (id); + + +-- +-- Name: events_archive_old_pkey; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX events_archive_old_pkey ON public.events_archive_old USING btree (region); + + +-- +-- Name: events_leaf_id_idx; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX events_leaf_id_idx ON public.events_leaf USING btree (id); + + +-- +-- Name: events_archive_id_idx; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_leaf_pkey ATTACH PARTITION public.events_archive_id_idx; + + +-- +-- Name: events_archive_old_id_idx; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_archive_id_idx ATTACH PARTITION public.events_archive_old_id_idx; + + +-- +-- Name: events_archive_old_pk; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_archive_pkey ATTACH PARTITION public.events_archive_old_pk; + + +-- +-- Name: events_archive_old_region_id_key; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_archive_region_id_key ATTACH PARTITION public.events_archive_old_region_id_key; + + +-- +-- Name: events_archive_pkey; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_pkey ATTACH PARTITION public.events_archive_pkey; + + +-- +-- Name: events_archive_region_id_key; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_region_id_key ATTACH PARTITION public.events_archive_region_id_key; + + +-- +-- Name: events_leaf_id_idx; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_leaf_pkey ATTACH PARTITION public.events_leaf_id_idx; + + +-- +-- Name: events_leaf_pkey1; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_pkey ATTACH PARTITION public.events_leaf_pkey1; + + +-- +-- Name: events_leaf_region_id_key; Type: INDEX ATTACH; Schema: public; Owner: - +-- + +ALTER INDEX public.events_region_id_key ATTACH PARTITION public.events_leaf_region_id_key; + + +-- +-- Name: event_notes event_notes_event_id_fkey; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.event_notes + ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES public.events(id); + + +-- +-- PostgreSQL database dump complete +-- + + diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql b/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql new file mode 100644 index 000000000..c19923e70 --- /dev/null +++ b/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql @@ -0,0 +1,123 @@ +-- +-- pgschema database dump +-- + +-- Dumped from database version PostgreSQL 17.6 +-- Dumped by pgschema version 1.13.3 + + +-- +-- Name: events; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + region text NOT NULL +) PARTITION BY RANGE (id); + +-- +-- Name: event_notes; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +-- +-- Name: events_archive; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS events_archive PARTITION OF events FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id); + +-- +-- Name: events_archive_old; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS events_archive_old PARTITION OF events_archive FOR VALUES FROM (10) TO (15); + +-- +-- Name: events_archive_old_id_idx; Type: INDEX; Schema: -; Owner: - +-- + +CREATE INDEX IF NOT EXISTS events_archive_old_id_idx ON events_archive_old (id); + +-- +-- Name: events_archive_old_pkey; Type: INDEX; Schema: -; Owner: - +-- + +CREATE INDEX IF NOT EXISTS events_archive_old_pkey ON events_archive_old (region); + +-- +-- Name: events_leaf; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) TO (10); + +-- +-- Name: events_leaf_id_idx; Type: INDEX; Schema: -; Owner: - +-- + +CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); + +-- +-- Name: events_pkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_pk PRIMARY KEY (id); +ALTER INDEX events_archive_pkey ATTACH PARTITION events_archive_old_pk; +ALTER INDEX events_pkey ATTACH PARTITION events_archive_pkey; +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey1 PRIMARY KEY (id); +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey1; + +-- +-- Name: events_region_id_key; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY events ADD CONSTRAINT events_region_id_key UNIQUE (region, id); +ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_region_id_key UNIQUE (region, id); +ALTER TABLE ONLY events_archive_old ADD CONSTRAINT events_archive_old_region_id_key UNIQUE (region, id); +ALTER INDEX events_archive_region_id_key ATTACH PARTITION events_archive_old_region_id_key; +ALTER INDEX events_region_id_key ATTACH PARTITION events_archive_region_id_key; +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id); +ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; + +-- +-- Name: events_archive_id_idx; Type: INDEX; Schema: -; Owner: - +-- + +CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); + +-- +-- Name: events_archive_id_idx; Type: INDEX; Schema: -; Owner: - +-- + +ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; + +-- +-- Name: events_leaf_pkey; Type: INDEX; Schema: -; Owner: - +-- + +CREATE INDEX IF NOT EXISTS events_leaf_pkey ON ONLY events (id); + +-- +-- Name: events_leaf_pkey; Type: INDEX; Schema: -; Owner: - +-- + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_archive_id_idx; + +-- +-- Name: events_leaf_pkey; Type: INDEX; Schema: -; Owner: - +-- + +ALTER INDEX events_leaf_pkey ATTACH PARTITION events_leaf_id_idx; + +-- +-- Name: event_notes_event_id_fkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql b/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql new file mode 100644 index 000000000..04b0a5059 --- /dev/null +++ b/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql @@ -0,0 +1,40 @@ +-- +-- Test case for GitHub issue #653: inherited constraint index names collide +-- with standalone indexes on replay +-- +-- PARTITION OF copies the parent's PRIMARY KEY and UNIQUE constraints onto +-- the new partition under generated names. When a standalone index already +-- holds such a name, PostgreSQL picks the next free one (events_leaf_pkey1). +-- A dump that creates the partition with the constraint inline, before the +-- standalone index, hands the generated name to the copy; the later +-- CREATE INDEX IF NOT EXISTS is skipped and ATTACH PARTITION fails. +-- +-- The dump therefore adds these constraints after the tables with +-- ALTER TABLE ONLY, each partition copy under its catalog name, and attaches +-- the copies explicitly. +-- + +CREATE TABLE events ( + id integer PRIMARY KEY, + region text NOT NULL, + CONSTRAINT events_region_id_key UNIQUE (region, id) +) PARTITION BY RANGE (id); + +CREATE INDEX events_leaf_pkey ON events (id); + +CREATE TABLE events_leaf PARTITION OF events FOR VALUES FROM (0) TO (10); + +CREATE TABLE events_archive PARTITION OF events + FOR VALUES FROM (10) TO (20) PARTITION BY RANGE (id); + +CREATE TABLE events_archive_old PARTITION OF events_archive FOR VALUES FROM (10) TO (15); + +-- A renamed copy whose original name now belongs to an index on another +-- column. Replaying with the constraint inline silently dropped this index. +ALTER INDEX events_archive_old_pkey RENAME TO events_archive_old_pk; +CREATE INDEX events_archive_old_pkey ON events_archive_old (region); + +CREATE TABLE event_notes ( + event_id integer NOT NULL REFERENCES events (id), + note text +); diff --git a/testdata/dump/sakila/pgschema.sql b/testdata/dump/sakila/pgschema.sql index 5ed343f61..9df3f6239 100644 --- a/testdata/dump/sakila/pgschema.sql +++ b/testdata/dump/sakila/pgschema.sql @@ -287,13 +287,12 @@ CREATE TABLE IF NOT EXISTS film_category ( -- CREATE TABLE IF NOT EXISTS payment ( - payment_id integer DEFAULT nextval('payment_payment_id_seq'::regclass), + payment_id integer DEFAULT nextval('payment_payment_id_seq'::regclass) NOT NULL, customer_id integer NOT NULL, staff_id integer NOT NULL, rental_id integer NOT NULL, amount numeric(5,2) NOT NULL, - payment_date timestamptz, - CONSTRAINT payment_pkey PRIMARY KEY (payment_date, payment_id) + payment_date timestamptz NOT NULL ) PARTITION BY RANGE (payment_date); -- @@ -891,6 +890,26 @@ RETURN; END $_$; +-- +-- Name: payment_pkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY payment ADD CONSTRAINT payment_pkey PRIMARY KEY (payment_date, payment_id); +ALTER TABLE ONLY payment_p2022_01 ADD CONSTRAINT payment_p2022_01_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_01_pkey; +ALTER TABLE ONLY payment_p2022_02 ADD CONSTRAINT payment_p2022_02_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_02_pkey; +ALTER TABLE ONLY payment_p2022_03 ADD CONSTRAINT payment_p2022_03_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_03_pkey; +ALTER TABLE ONLY payment_p2022_04 ADD CONSTRAINT payment_p2022_04_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_04_pkey; +ALTER TABLE ONLY payment_p2022_05 ADD CONSTRAINT payment_p2022_05_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_05_pkey; +ALTER TABLE ONLY payment_p2022_06 ADD CONSTRAINT payment_p2022_06_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_06_pkey; +ALTER TABLE ONLY payment_p2022_07 ADD CONSTRAINT payment_p2022_07_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_07_pkey; + -- -- Name: group_concat(text); Type: AGGREGATE; Schema: -; Owner: - -- From f44bccc9cdf764e240ad9677512678ff954ddcd9 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 26 Oct 2026 20:37:55 -0700 Subject: [PATCH 2/4] fix: add postponed partition keys before functions that depend on them Review of #663 found that SQL-language functions whose bodies are validated at creation (BEGIN ATOMIC, or check_function_bodies on) and depend on a postponed key, such as ON CONFLICT ON CONSTRAINT events_pkey, were created before the key existed. Each constraint family is now added inside the table batch as soon as its last partition is created, so it precedes every function scheduled after that batch. A hierarchy with postponed keys is created in a single batch, the latest any of its tables needs. The flush before the standalone partitioned indexes remains for anything not emitted that way. The diff fixture, the dump fixture and the partition identity test gain a BEGIN ATOMIC function with ON CONFLICT ON CONSTRAINT on the partitioned key. Co-Authored-By: Claude Fable 5.1 --- cmd/dump/partition_index_integration_test.go | 4 + internal/diff/diff.go | 36 +++++- internal/diff/partition_index.go | 105 +++++++++++++++++- internal/diff/table.go | 1 + .../diff.sql | 20 +++- .../new.sql | 8 ++ .../plan.json | 18 ++- .../plan.sql | 20 +++- .../plan.txt | 26 ++++- .../pgschema.sql | 16 +-- .../pgdump.sql | 12 ++ .../pgschema.sql | 16 +++ .../raw.sql | 9 ++ testdata/dump/sakila/pgschema.sql | 40 +++---- 14 files changed, 275 insertions(+), 56 deletions(-) diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go index 01a23ed1d..1add54c94 100644 --- a/cmd/dump/partition_index_integration_test.go +++ b/cmd/dump/partition_index_integration_test.go @@ -89,6 +89,10 @@ CREATE TABLE keyed_events_late_a PARTITION OF keyed_events_late FOR VALUES FROM ALTER INDEX keyed_events_late_a_pkey RENAME TO keyed_late_a_pk; CREATE INDEX keyed_events_late_a_pkey ON keyed_events_late_a (note); CREATE TABLE keyed_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCES keyed_events (bucket, id)); +-- A SQL-standard body is validated at creation, so the key must already exist. +CREATE FUNCTION upsert_keyed_event(p_bucket int, p_id int) RETURNS void LANGUAGE sql BEGIN ATOMIC + INSERT INTO keyed_events (bucket, id) VALUES (p_bucket, p_id) ON CONFLICT ON CONSTRAINT keyed_events_pkey DO NOTHING; +END; ` for _, constraintType := range []string{"UNIQUE", "PRIMARY KEY"} { for _, multiFile := range []bool{false, true} { diff --git a/internal/diff/diff.go b/internal/diff/diff.go index 8a5a430c8..f2e207bc9 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -2020,12 +2020,39 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto tablesWithDeps := []*ir.Table{} tablesAfterTableDomains := []*ir.Table{} - for _, table := range d.addedTables { + tableBatch := func(table *ir.Table) int { if tableUsesDeferredDomain(table, tableDomainLookup) { + return 2 + } + if tableReferencesNewFunction(table, newFunctionLookup) || tableUsesDeferredDomain(table, functionDomainLookup) { + return 1 + } + return 0 + } + batches := make(map[string]int, len(d.addedTables)) + // A hierarchy whose keys are postponed until its last partition exists + // (issue #653) is created in one batch, the latest any member needs, so + // its keys still precede the functions scheduled after that batch. + hierarchyBatch := make(map[string]int) + for _, table := range d.addedTables { + key := table.Schema + "." + table.Name + batches[key] = tableBatch(table) + if root := d.partitionIndexes.hierarchyRoot(table); root != "" && batches[key] > hierarchyBatch[root] { + hierarchyBatch[root] = batches[key] + } + } + for _, table := range d.addedTables { + key := table.Schema + "." + table.Name + batch := batches[key] + if root := d.partitionIndexes.hierarchyRoot(table); root != "" { + batch = hierarchyBatch[root] + } + switch batch { + case 2: tablesAfterTableDomains = append(tablesAfterTableDomains, table) - } else if tableReferencesNewFunction(table, newFunctionLookup) || tableUsesDeferredDomain(table, functionDomainLookup) { + case 1: tablesWithDeps = append(tablesWithDeps, table) - } else { + default: tablesWithoutDeps = append(tablesWithoutDeps, table) } } @@ -2153,7 +2180,8 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateSequenceComment(seq, targetSchema, DiffOperationCreate, collector) } - // Postponed PRIMARY KEY and UNIQUE constraints of partitioned tables come + // Postponed PRIMARY KEY and UNIQUE constraints are normally added inside + // the table batches as each hierarchy completes; any remaining family comes // before the standalone partitioned indexes, which may attach to them. d.partitionIndexes.generateDeferredConstraintSQL(targetSchema, collector) d.partitionIndexes.generateSQL(targetSchema, collector) diff --git a/internal/diff/partition_index.go b/internal/diff/partition_index.go index b44fe9d12..603d9acc9 100644 --- a/internal/diff/partition_index.go +++ b/internal/diff/partition_index.go @@ -119,6 +119,14 @@ type partitionIndexCreationPlan struct { // partitions lists the added partitions of each table key, in name order. partitions map[string][]*ir.Table tables map[string]*ir.Table + // roots maps each table of a hierarchy with postponed constraints to the + // hierarchy's root key, so the whole hierarchy is created in one batch. + roots map[string]string + // created and emitted track creation progress so each constraint family + // is added as soon as its last partition exists, ahead of SQL functions + // whose bodies depend on the key. + created map[string]bool + emitted map[string]map[string]bool } func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[string]*ir.Table, dump bool) *partitionIndexCreationPlan { @@ -128,6 +136,9 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin uniqueTables: make(map[string]bool), constraints: make(map[string]map[string]bool), partitions: make(map[string][]*ir.Table), + roots: make(map[string]string), + created: make(map[string]bool), + emitted: make(map[string]map[string]bool), } added := make(map[string]*ir.Table) available := availablePartitionIndexes(addedTables) @@ -228,6 +239,25 @@ func planPartitionIndexCreation(addedTables []*ir.Table, desiredTables map[strin plan.markUniqueTables(table) } } + for key := range plan.constraints { + root := key + for { + table := added[root] + if table == nil || table.PartitionOf == "" { + break + } + parentSchema := table.PartitionOfSchema + if parentSchema == "" { + parentSchema = table.Schema + } + parentKey := parentSchema + "." + table.PartitionOf + if added[parentKey] == nil { + break + } + root = parentKey + } + plan.markHierarchy(added[root], root) + } for parent, index := range plan.indexes { plan.children[parent] = make(map[ir.IndexRef]bool) for _, child := range index.AttachedPartitions { @@ -282,6 +312,23 @@ func (p *partitionIndexCreationPlan) inheritedNamesKnown(table *ir.Table, constr return true } +func (p *partitionIndexCreationPlan) markHierarchy(table *ir.Table, root string) { + key := table.Schema + "." + table.Name + p.roots[key] = root + for _, child := range p.partitions[key] { + p.markHierarchy(child, root) + } +} + +// hierarchyRoot returns the root key of a hierarchy with postponed +// constraints, or "" for any other table. +func (p *partitionIndexCreationPlan) hierarchyRoot(table *ir.Table) string { + if p == nil { + return "" + } + return p.roots[table.Schema+"."+table.Name] +} + func (p *partitionIndexCreationPlan) markUniqueTables(table *ir.Table) { key := table.Schema + "." + table.Name p.uniqueTables[key] = true @@ -299,15 +346,38 @@ func (p *partitionIndexCreationPlan) defersConstraint(table *ir.Table, constrain return p.constraints[table.Schema+"."+table.Name][constraint.Name] } -// generateDeferredConstraintSQL adds the postponed PRIMARY KEY and UNIQUE -// constraints once every partition exists. ONLY keeps PostgreSQL from creating -// partition copies under generated names; each copy is added under its catalog -// name and attached explicitly. One step carries the whole family so the -// statements stay together in plans and multi-file dumps. +// tableCreated records that the table's CREATE TABLE was emitted and adds the +// postponed constraints of every family that is now complete. Emitting them +// here, inside the table batch, keeps them ahead of SQL functions created +// after the batch whose bodies depend on the key (ON CONFLICT ON CONSTRAINT, +// GROUP BY functional dependency). +func (p *partitionIndexCreationPlan) tableCreated(table *ir.Table, targetSchema string, collector *diffCollector) { + if p == nil { + return + } + p.created[table.Schema+"."+table.Name] = true + if p.hierarchyRoot(table) == "" { + return + } + p.generateReadyConstraintSQL(targetSchema, collector, false) +} + +// generateDeferredConstraintSQL adds any postponed constraint family not yet +// emitted by tableCreated. func (p *partitionIndexCreationPlan) generateDeferredConstraintSQL(targetSchema string, collector *diffCollector) { if p == nil { return } + p.generateReadyConstraintSQL(targetSchema, collector, true) +} + +// generateReadyConstraintSQL adds the postponed PRIMARY KEY and UNIQUE +// constraints of each family whose table and partitions all exist (or of every +// remaining family when flush is set). ONLY keeps PostgreSQL from creating +// partition copies under generated names; each copy is added under its catalog +// name and attached explicitly. One step carries the whole family so the +// statements stay together in plans and multi-file dumps. +func (p *partitionIndexCreationPlan) generateReadyConstraintSQL(targetSchema string, collector *diffCollector, flush bool) { keys := make([]string, 0, len(p.constraints)) for key := range p.constraints { keys = append(keys, key) @@ -315,7 +385,17 @@ func (p *partitionIndexCreationPlan) generateDeferredConstraintSQL(targetSchema sort.Strings(keys) for _, key := range keys { table := p.tables[key] + if !flush && !p.hierarchyCreated(table) { + continue + } for _, name := range sortedKeys(p.constraints[key]) { + if p.emitted[key][name] { + continue + } + if p.emitted[key] == nil { + p.emitted[key] = make(map[string]bool) + } + p.emitted[key][name] = true constraint := table.Constraints[name] var statements []SQLStatement p.appendConstraintFamily(&statements, table, constraint, targetSchema, collector.qualifySchema) @@ -328,6 +408,21 @@ func (p *partitionIndexCreationPlan) generateDeferredConstraintSQL(targetSchema } } +// hierarchyCreated reports whether the table and every partition below it +// have been created. +func (p *partitionIndexCreationPlan) hierarchyCreated(table *ir.Table) bool { + key := table.Schema + "." + table.Name + if !p.created[key] { + return false + } + for _, child := range p.partitions[key] { + if !p.hierarchyCreated(child) { + return false + } + } + return true +} + // appendConstraintFamily adds the constraint to its table with ONLY, then each // partition's copy under its recorded name, attaching every copy to the // constraint's index. Nested partitions attach to their immediate parent. diff --git a/internal/diff/table.go b/internal/diff/table.go index e36ad7ddf..7a4aecc17 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -590,6 +590,7 @@ func generateCreateTablesSQL( } createdTables[fmt.Sprintf("%s.%s", table.Schema, table.Name)] = true + partitionIndexes.tableCreated(table, targetSchema, collector) } return deferredPolicies, deferredConstraints diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql index d7547f1e6..1ae7dfa41 100644 --- a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/diff.sql @@ -18,10 +18,6 @@ CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) T CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); -CREATE TABLE IF NOT EXISTS leaf_notes ( - event_id integer NOT NULL -); - ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); @@ -50,6 +46,22 @@ ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (re ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; +CREATE TABLE IF NOT EXISTS leaf_notes ( + event_id integer NOT NULL +); + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_region text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO events (id, region) + VALUES (upsert_event.p_id, upsert_event.p_region) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = excluded.region; +END; + CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql index 4eb952247..d05e39c84 100644 --- a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/new.sql @@ -29,3 +29,11 @@ CREATE TABLE public.event_notes ( CREATE TABLE public.leaf_notes ( event_id integer NOT NULL REFERENCES public.events_leaf (id) ); + +-- A SQL-standard function body is validated at creation and needs the key. +CREATE FUNCTION public.upsert_event(p_id integer, p_region text) RETURNS void +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO public.events (id, region) VALUES (p_id, p_region) + ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = EXCLUDED.region; +END; diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json index a269412be..ddc27e193 100644 --- a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.json @@ -50,12 +50,6 @@ "operation": "create", "path": "public.events_leaf.events_leaf_id_idx" }, - { - "sql": "CREATE TABLE IF NOT EXISTS leaf_notes (\n event_id integer NOT NULL\n);", - "type": "table", - "operation": "create", - "path": "public.leaf_notes" - }, { "sql": "ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id);", "type": "table.constraint", @@ -140,6 +134,18 @@ "operation": "create", "path": "public.events.events_region_id_key" }, + { + "sql": "CREATE TABLE IF NOT EXISTS leaf_notes (\n event_id integer NOT NULL\n);", + "type": "table", + "operation": "create", + "path": "public.leaf_notes" + }, + { + "sql": "CREATE OR REPLACE FUNCTION upsert_event(\n p_id integer,\n p_region text\n)\nRETURNS void\nLANGUAGE sql\nVOLATILE\nBEGIN ATOMIC\n INSERT INTO public.events (id, region)\n VALUES (upsert_event.p_id, upsert_event.p_region) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = excluded.region;\nEND;", + "type": "function", + "operation": "create", + "path": "public.upsert_event" + }, { "sql": "CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id);", "type": "table.index", diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql index d7547f1e6..df6bb5ff1 100644 --- a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.sql @@ -18,10 +18,6 @@ CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) T CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); -CREATE TABLE IF NOT EXISTS leaf_notes ( - event_id integer NOT NULL -); - ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); @@ -50,6 +46,22 @@ ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (re ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; +CREATE TABLE IF NOT EXISTS leaf_notes ( + event_id integer NOT NULL +); + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_region text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO public.events (id, region) + VALUES (upsert_event.p_id, upsert_event.p_region) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = excluded.region; +END; + CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; diff --git a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt index 0a125c0ea..20db0b399 100644 --- a/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt +++ b/testdata/diff/create_table/issue_653_inherited_constraint_index_name/plan.txt @@ -1,8 +1,12 @@ -Plan: 6 to add. +Plan: 7 to add. Summary by type: + functions: 1 to add tables: 6 to add +Functions: + + upsert_event + Tables: + event_notes + event_notes_event_id_fkey (constraint) @@ -44,10 +48,6 @@ CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events FOR VALUES FROM (0) T CREATE INDEX IF NOT EXISTS events_leaf_id_idx ON events_leaf (id); -CREATE TABLE IF NOT EXISTS leaf_notes ( - event_id integer NOT NULL -); - ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); ALTER TABLE ONLY events_archive ADD CONSTRAINT events_archive_pkey PRIMARY KEY (id); @@ -76,6 +76,22 @@ ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (re ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; +CREATE TABLE IF NOT EXISTS leaf_notes ( + event_id integer NOT NULL +); + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_region text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO public.events (id, region) + VALUES (upsert_event.p_id, upsert_event.p_region) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = excluded.region; +END; + CREATE INDEX IF NOT EXISTS events_archive_id_idx ON ONLY events_archive (id); ALTER INDEX events_archive_id_idx ATTACH PARTITION events_archive_old_id_idx; diff --git a/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql b/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql index c8acebb24..d967d1c78 100644 --- a/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql +++ b/testdata/dump/issue_472_partition_clone_trigger/pgschema.sql @@ -22,6 +22,14 @@ CREATE TABLE IF NOT EXISTS ledger ( CREATE TABLE IF NOT EXISTS ledger_2026_06 PARTITION OF ledger FOR VALUES FROM ('2026-06-01 00:00:00+00') TO ('2026-07-01 00:00:00+00'); +-- +-- Name: ledger_pkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY ledger ADD CONSTRAINT ledger_pkey PRIMARY KEY (ts, id); +ALTER TABLE ONLY ledger_2026_06 ADD CONSTRAINT ledger_2026_06_pkey PRIMARY KEY (ts, id); +ALTER INDEX ledger_pkey ATTACH PARTITION ledger_2026_06_pkey; + -- -- Name: tg_noop(); Type: FUNCTION; Schema: -; Owner: - -- @@ -33,14 +41,6 @@ VOLATILE AS $$ BEGIN RETURN NEW; END $$; --- --- Name: ledger_pkey; Type: CONSTRAINT; Schema: -; Owner: - --- - -ALTER TABLE ONLY ledger ADD CONSTRAINT ledger_pkey PRIMARY KEY (ts, id); -ALTER TABLE ONLY ledger_2026_06 ADD CONSTRAINT ledger_2026_06_pkey PRIMARY KEY (ts, id); -ALTER INDEX ledger_pkey ATTACH PARTITION ledger_2026_06_pkey; - -- -- Name: trg_rollup; Type: TRIGGER; Schema: -; Owner: - -- diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql b/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql index da1d7912b..c87a52253 100644 --- a/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql +++ b/testdata/dump/issue_653_inherited_constraint_index_name/pgdump.sql @@ -103,6 +103,18 @@ ALTER TABLE ONLY public.events ADD CONSTRAINT events_pkey PRIMARY KEY (id); +-- +-- Name: upsert_event(integer, text); Type: FUNCTION; Schema: public; Owner: - +-- + +CREATE FUNCTION public.upsert_event(p_id integer, p_region text) RETURNS void + LANGUAGE sql + BEGIN ATOMIC + INSERT INTO public.events (id, region) + VALUES (upsert_event.p_id, upsert_event.p_region) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = excluded.region; +END; + + -- -- Name: events_archive events_archive_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql b/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql index c19923e70..43ed727b4 100644 --- a/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql +++ b/testdata/dump/issue_653_inherited_constraint_index_name/pgschema.sql @@ -84,6 +84,22 @@ ALTER INDEX events_region_id_key ATTACH PARTITION events_archive_region_id_key; ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_region_id_key UNIQUE (region, id); ALTER INDEX events_region_id_key ATTACH PARTITION events_leaf_region_id_key; +-- +-- Name: upsert_event(integer, text); Type: FUNCTION; Schema: -; Owner: - +-- + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_region text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO events (id, region) + VALUES (upsert_event.p_id, upsert_event.p_region) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = excluded.region; +END; + -- -- Name: events_archive_id_idx; Type: INDEX; Schema: -; Owner: - -- diff --git a/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql b/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql index 04b0a5059..7067b642c 100644 --- a/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql +++ b/testdata/dump/issue_653_inherited_constraint_index_name/raw.sql @@ -38,3 +38,12 @@ CREATE TABLE event_notes ( event_id integer NOT NULL REFERENCES events (id), note text ); + +-- A SQL-standard function body is validated at creation and needs the key, +-- so the dump adds the constraints before functions that use the table. +CREATE FUNCTION upsert_event(p_id integer, p_region text) RETURNS void +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO events (id, region) VALUES (p_id, p_region) + ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET region = EXCLUDED.region; +END; diff --git a/testdata/dump/sakila/pgschema.sql b/testdata/dump/sakila/pgschema.sql index 9df3f6239..d3a8b3b2a 100644 --- a/testdata/dump/sakila/pgschema.sql +++ b/testdata/dump/sakila/pgschema.sql @@ -619,6 +619,26 @@ CREATE INDEX IF NOT EXISTS idx_fk_payment_p2022_07_staff_id ON payment_p2022_07 CREATE INDEX IF NOT EXISTS payment_p2022_07_customer_id_idx ON payment_p2022_07 (customer_id); +-- +-- Name: payment_pkey; Type: CONSTRAINT; Schema: -; Owner: - +-- + +ALTER TABLE ONLY payment ADD CONSTRAINT payment_pkey PRIMARY KEY (payment_date, payment_id); +ALTER TABLE ONLY payment_p2022_01 ADD CONSTRAINT payment_p2022_01_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_01_pkey; +ALTER TABLE ONLY payment_p2022_02 ADD CONSTRAINT payment_p2022_02_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_02_pkey; +ALTER TABLE ONLY payment_p2022_03 ADD CONSTRAINT payment_p2022_03_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_03_pkey; +ALTER TABLE ONLY payment_p2022_04 ADD CONSTRAINT payment_p2022_04_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_04_pkey; +ALTER TABLE ONLY payment_p2022_05 ADD CONSTRAINT payment_p2022_05_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_05_pkey; +ALTER TABLE ONLY payment_p2022_06 ADD CONSTRAINT payment_p2022_06_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_06_pkey; +ALTER TABLE ONLY payment_p2022_07 ADD CONSTRAINT payment_p2022_07_pkey PRIMARY KEY (payment_date, payment_id); +ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_07_pkey; + -- -- Name: _group_concat(text, text); Type: FUNCTION; Schema: -; Owner: - -- @@ -890,26 +910,6 @@ RETURN; END $_$; --- --- Name: payment_pkey; Type: CONSTRAINT; Schema: -; Owner: - --- - -ALTER TABLE ONLY payment ADD CONSTRAINT payment_pkey PRIMARY KEY (payment_date, payment_id); -ALTER TABLE ONLY payment_p2022_01 ADD CONSTRAINT payment_p2022_01_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_01_pkey; -ALTER TABLE ONLY payment_p2022_02 ADD CONSTRAINT payment_p2022_02_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_02_pkey; -ALTER TABLE ONLY payment_p2022_03 ADD CONSTRAINT payment_p2022_03_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_03_pkey; -ALTER TABLE ONLY payment_p2022_04 ADD CONSTRAINT payment_p2022_04_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_04_pkey; -ALTER TABLE ONLY payment_p2022_05 ADD CONSTRAINT payment_p2022_05_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_05_pkey; -ALTER TABLE ONLY payment_p2022_06 ADD CONSTRAINT payment_p2022_06_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_06_pkey; -ALTER TABLE ONLY payment_p2022_07 ADD CONSTRAINT payment_p2022_07_pkey PRIMARY KEY (payment_date, payment_id); -ALTER INDEX payment_pkey ATTACH PARTITION payment_p2022_07_pkey; - -- -- Name: group_concat(text); Type: AGGREGATE; Schema: -; Owner: - -- From 9c9872783022978c64f062c7bd7f12ea6339df61 Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 26 Oct 2026 21:56:37 -0700 Subject: [PATCH 3/4] fix: keep dependency order around postponed partition keys Review of #663 found two ordering problems with the postponed keys. Creating a whole hierarchy in its latest batch broke a valid chain: a partitioned parent, a SQL function reading it, and a partition whose default calls that function. Tables now keep their own batches. Instead, a function referencing a hierarchy that completes in a later batch moves after that batch, since its body may depend on the key, unless a table default or CHECK or a domain created earlier needs it; then the parent -> function -> partition order wins. Policies on tables of such a hierarchy were created with their table, before the keys, so a policy expression relying on the key (GROUP BY functional dependency) failed to parse. They now follow the keys. Multi-file dumps keep them after the constraint file under policies/ rather than bundling them back into the table file. A new diff fixture and the partition identity test cover the chain, the held-back function and the policy. Co-Authored-By: Claude Fable 5.1 --- cmd/dump/partition_index_integration_test.go | 9 ++ docs/cli/dump.mdx | 4 +- internal/diff/diff.go | 83 +++++++++++++++---- internal/diff/table.go | 4 +- internal/dump/formatter.go | 36 ++++++++ .../diff.sql | 39 +++++++++ .../new.sql | 31 +++++++ .../old.sql | 1 + .../plan.json | 68 +++++++++++++++ .../plan.sql | 39 +++++++++ .../plan.txt | 59 +++++++++++++ 11 files changed, 356 insertions(+), 17 deletions(-) create mode 100644 testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql create mode 100644 testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql create mode 100644 testdata/diff/create_table/issue_653_partition_default_function_chain/old.sql create mode 100644 testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json create mode 100644 testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql create mode 100644 testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go index 1add54c94..c27484d4e 100644 --- a/cmd/dump/partition_index_integration_test.go +++ b/cmd/dump/partition_index_integration_test.go @@ -93,6 +93,15 @@ CREATE TABLE keyed_links (bucket int, id int, FOREIGN KEY (bucket, id) REFERENCE CREATE FUNCTION upsert_keyed_event(p_bucket int, p_id int) RETURNS void LANGUAGE sql BEGIN ATOMIC INSERT INTO keyed_events (bucket, id) VALUES (p_bucket, p_id) ON CONFLICT ON CONSTRAINT keyed_events_pkey DO NOTHING; END; +-- A partition default calling a function that reads the parent keeps the +-- parent, the function and the partition in that order. +CREATE FUNCTION count_keyed_events() RETURNS text LANGUAGE sql BEGIN ATOMIC + SELECT count(*)::text FROM keyed_events; +END; +CREATE TABLE keyed_events_rest PARTITION OF keyed_events (note DEFAULT count_keyed_events()) DEFAULT; +-- A policy expression relying on the key follows the postponed constraints. +ALTER TABLE keyed_events ENABLE ROW LEVEL SECURITY; +CREATE POLICY keyed_events_grouped ON keyed_events USING (EXISTS (SELECT note FROM keyed_events GROUP BY bucket, id)); ` for _, constraintType := range []string{"UNIQUE", "PRIMARY KEY"} { for _, multiFile := range []bool{false, true} { diff --git a/docs/cli/dump.mdx b/docs/cli/dump.mdx index c4442d4d0..a1c1f6294 100644 --- a/docs/cli/dump.mdx +++ b/docs/cli/dump.mdx @@ -270,7 +270,9 @@ Instead, `pgschema` adds the constraint after the tables with `ALTER TABLE ONLY ... ADD CONSTRAINT`, adds each partition's copy under its original name the same way, and attaches the copies with `ALTER INDEX ... ATTACH PARTITION`. Multi-file dumps write each constraint and -its copies under `constraints/`, such as `constraints/0001_events.sql`. +its copies under `constraints/`, such as `constraints/0001_events.sql`. Row-level +security policies on such a table follow its constraints, under `policies/` in +multi-file dumps, because a policy expression may rely on the key. Each individual file (e.g., `tables/users.sql`) contains the specific object definition: ```sql diff --git a/internal/diff/diff.go b/internal/diff/diff.go index f2e207bc9..edd4d4518 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -2030,24 +2030,10 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto return 0 } batches := make(map[string]int, len(d.addedTables)) - // A hierarchy whose keys are postponed until its last partition exists - // (issue #653) is created in one batch, the latest any member needs, so - // its keys still precede the functions scheduled after that batch. - hierarchyBatch := make(map[string]int) for _, table := range d.addedTables { key := table.Schema + "." + table.Name batches[key] = tableBatch(table) - if root := d.partitionIndexes.hierarchyRoot(table); root != "" && batches[key] > hierarchyBatch[root] { - hierarchyBatch[root] = batches[key] - } - } - for _, table := range d.addedTables { - key := table.Schema + "." + table.Name - batch := batches[key] - if root := d.partitionIndexes.hierarchyRoot(table); root != "" { - batch = hierarchyBatch[root] - } - switch batch { + switch batches[key] { case 2: tablesAfterTableDomains = append(tablesAfterTableDomains, table) case 1: @@ -2139,6 +2125,11 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto functionsWithoutTableDeps = append(functionsWithoutTableDeps, fn) } } + // A function whose body may depend on a postponed key (issue #653) is + // validated at creation, so one referencing a hierarchy that completes + // in a later batch moves after that batch, unless an earlier table or + // domain needs it first. + functionsWithoutTableDeps, functionsWithTableDeps, functionsAfterAllTables = holdBackKeyDependentFunctions(d.partitionIndexes, batches, d.addedTables, domainsWithFunctionDepsOnly, functionsWithoutTableDeps, functionsWithTableDeps, functionsAfterAllTables) // A SQL function calling a function from a later batch is validated against // it at creation, so it moves to that batch even if it touches no new table. functionsWithoutTableDeps, functionsWithTableDeps, functionsAfterAllTables = holdBackCallersAcrossBatches(functionsWithoutTableDeps, functionsWithTableDeps, functionsAfterAllTables) @@ -3072,6 +3063,68 @@ func splitFunctionsCallingAggregates(functions []*ir.Function, aggregates []*ir. // holdBackCallersAcrossBatches applies holdBackCallers to three consecutive function // batches until none changes. A single round is not enough: a function that joins a // later batch in one step can strand a caller that an earlier step already examined. +// holdBackKeyDependentFunctions moves a function that references a table of a +// hierarchy with postponed keys (issue #653) to the function batch after the +// table batch in which that hierarchy completes. Whether the body depends on +// the key (ON CONFLICT ON CONSTRAINT, GROUP BY functional dependency) is not +// known, so every such function moves unless a table default or CHECK, or a +// domain created earlier, needs it: then the parent -> function -> partition +// chain keeps its order. batches maps table keys to their creation batch; +// first, second and third are the function batches created after table +// batches 0, 1 and 2. +func holdBackKeyDependentFunctions(plan *partitionIndexCreationPlan, batches map[string]int, addedTables []*ir.Table, domains []*ir.Type, first, second, third []*ir.Function) ([]*ir.Function, []*ir.Function, []*ir.Function) { + if plan == nil || len(plan.roots) == 0 { + return first, second, third + } + completion := make(map[string]int) + lookups := make(map[string]map[string]struct{}) + for _, table := range addedTables { + root := plan.hierarchyRoot(table) + if root == "" { + continue + } + key := table.Schema + "." + table.Name + if batches[key] > completion[root] { + completion[root] = batches[key] + } + if lookups[root] == nil { + lookups[root] = make(map[string]struct{}) + } + lookups[root][strings.ToLower(key)] = struct{}{} + lookups[root][strings.ToLower(table.Name)] = struct{}{} + } + neededEarlier := func(fn *ir.Function) bool { + lookup := buildFunctionLookup([]*ir.Function{fn}) + for _, table := range addedTables { + if tableReferencesNewFunction(table, lookup) { + return true + } + } + for _, domain := range domains { + if domainReferencesNewFunction(domain, lookup) { + return true + } + } + return false + } + var result [3][]*ir.Function + for batch, fns := range [][]*ir.Function{first, second, third} { + for _, fn := range fns { + target := batch + for root, lookup := range lookups { + if completion[root] > target && functionReferencesNewTable(fn, lookup) { + target = completion[root] + } + } + if target != batch && neededEarlier(fn) { + target = batch + } + result[target] = append(result[target], fn) + } + } + return result[0], result[1], result[2] +} + func holdBackCallersAcrossBatches(first, second, third []*ir.Function) ([]*ir.Function, []*ir.Function, []*ir.Function) { for { secondLen, thirdLen := len(second), len(third) diff --git a/internal/diff/table.go b/internal/diff/table.go index 7a4aecc17..df68f4960 100644 --- a/internal/diff/table.go +++ b/internal/diff/table.go @@ -577,7 +577,9 @@ func generateCreateTablesSQL( policyNames := sortedKeys(table.Policies) for _, name := range policyNames { policy := table.Policies[name] - if shouldDeferPolicy != nil && shouldDeferPolicy(policy) { + // A policy on a table whose keys are postponed may parse against + // them (GROUP BY functional dependency), so it follows them. + if (shouldDeferPolicy != nil && shouldDeferPolicy(policy)) || partitionIndexes.hierarchyRoot(table) != "" { deferredPolicies = append(deferredPolicies, policy) } else { inlinePolicies = append(inlinePolicies, policy) diff --git a/internal/dump/formatter.go b/internal/dump/formatter.go index 23637a8ca..30122dace 100644 --- a/internal/dump/formatter.go +++ b/internal/dump/formatter.go @@ -96,6 +96,30 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er partitionIndexFiles := make(map[indexKey]fileKey) var tables []*ir.Table var views []*ir.View + // Tables whose PRIMARY KEY or UNIQUE constraints were emitted as separate + // steps after the tables (partitioned tables, issue #653), with the tables + // seen so far for walking a partition up to such a table. + keyedTables := make(map[string]bool) + tablesByKey := make(map[string]*ir.Table) + policyIndex := 0 + policyFollowsKeys := func(policy *ir.RLSPolicy) bool { + key := policy.Schema + "." + policy.Table + for key != "" { + if keyedTables[key] { + return true + } + table := tablesByKey[key] + if table == nil || table.PartitionOf == "" { + return false + } + parentSchema := table.PartitionOfSchema + if parentSchema == "" { + parentSchema = table.Schema + } + key = parentSchema + "." + table.PartitionOf + } + return false + } for _, step := range diffs { var dir string @@ -115,6 +139,17 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er key = fileKey{dir: "constraints", name: fmt.Sprintf("%04d_%s", constraintIndex, key.name)} } + if constraint, ok := step.Source.(*ir.Constraint); ok && step.Type == diff.DiffTypeTableConstraint && + (constraint.Type == ir.ConstraintTypePrimaryKey || constraint.Type == ir.ConstraintTypeUnique) { + keyedTables[constraint.Schema+"."+constraint.Table] = true + } + if policy, ok := step.Source.(*ir.RLSPolicy); ok && step.Type == diff.DiffTypeTablePolicy && policyFollowsKeys(policy) { + // A policy created after its table's postponed keys stays after + // them instead of moving back into the table file. + policyIndex++ + key = fileKey{dir: "policies", name: fmt.Sprintf("%04d_%s", policyIndex, policy.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. @@ -137,6 +172,7 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er switch src := step.Source.(type) { case *ir.Table: tables = append(tables, src) + tablesByKey[src.Schema+"."+src.Name] = src case *ir.View: views = append(views, src) } diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql new file mode 100644 index 000000000..0060ae996 --- /dev/null +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql @@ -0,0 +1,39 @@ +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + payload text +) PARTITION BY RANGE (id); + +ALTER TABLE events ENABLE ROW LEVEL SECURITY; + +CREATE OR REPLACE FUNCTION count_events() +RETURNS text +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + SELECT (count(*))::text AS count + FROM events; +END; + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events ( + payload DEFAULT count_events() +) FOR VALUES FROM (0) TO (10); + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey PRIMARY KEY (id); + +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey; + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_payload text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO events (id, payload) + VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; +END; + +CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id)); diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql new file mode 100644 index 000000000..f73f00231 --- /dev/null +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql @@ -0,0 +1,31 @@ +-- The partition's default calls a SQL function that reads the parent, so the +-- parent, the function and the partition keep that order even though the +-- parent's key is postponed until the partition exists (issue #653). A +-- function that nothing else needs waits for the key, and so does the +-- parent's policy, whose expression relies on the key. +CREATE TABLE public.events ( + id integer PRIMARY KEY, + payload text +) PARTITION BY RANGE (id); + +CREATE FUNCTION public.count_events() RETURNS text +LANGUAGE sql +BEGIN ATOMIC + SELECT count(*)::text FROM public.events; +END; + +CREATE TABLE public.events_leaf PARTITION OF public.events ( + payload DEFAULT public.count_events() +) FOR VALUES FROM (0) TO (10); + +CREATE FUNCTION public.upsert_event(p_id integer, p_payload text) RETURNS void +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO public.events (id, payload) VALUES (p_id, p_payload) + ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = EXCLUDED.payload; +END; + +ALTER TABLE public.events ENABLE ROW LEVEL SECURITY; + +CREATE POLICY events_grouped ON public.events + USING (EXISTS (SELECT payload FROM public.events GROUP BY id)); diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/old.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/old.sql new file mode 100644 index 000000000..03d7e9aef --- /dev/null +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/old.sql @@ -0,0 +1 @@ +-- Empty schema diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json new file mode 100644 index 000000000..859a5a778 --- /dev/null +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json @@ -0,0 +1,68 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.13.3", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "965b1131737c955e24c7f827c55bd78e4cb49a75adfd04229e0ba297376f5085" + }, + "groups": [ + { + "steps": [ + { + "sql": "CREATE TABLE IF NOT EXISTS events (\n id integer NOT NULL,\n payload text\n) PARTITION BY RANGE (id);", + "type": "table", + "operation": "create", + "path": "public.events" + }, + { + "sql": "ALTER TABLE events ENABLE ROW LEVEL SECURITY;", + "type": "table.rls", + "operation": "alter", + "path": "public.events" + }, + { + "sql": "CREATE OR REPLACE FUNCTION count_events()\nRETURNS text\nLANGUAGE sql\nVOLATILE\nBEGIN ATOMIC\n SELECT (count(*))::text AS count\n FROM public.events;\nEND;", + "type": "function", + "operation": "create", + "path": "public.count_events" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events (\n payload DEFAULT count_events()\n) FOR VALUES FROM (0) TO (10);", + "type": "table", + "operation": "create", + "path": "public.events_leaf" + }, + { + "sql": "ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey PRIMARY KEY (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey;", + "type": "table.constraint", + "operation": "create", + "path": "public.events.events_pkey" + }, + { + "sql": "CREATE OR REPLACE FUNCTION upsert_event(\n p_id integer,\n p_payload text\n)\nRETURNS void\nLANGUAGE sql\nVOLATILE\nBEGIN ATOMIC\n INSERT INTO public.events (id, payload)\n VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload;\nEND;", + "type": "function", + "operation": "create", + "path": "public.upsert_event" + }, + { + "sql": "CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id));", + "type": "table.policy", + "operation": "create", + "path": "public.events.events_grouped" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql new file mode 100644 index 000000000..c63b0b198 --- /dev/null +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql @@ -0,0 +1,39 @@ +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + payload text +) PARTITION BY RANGE (id); + +ALTER TABLE events ENABLE ROW LEVEL SECURITY; + +CREATE OR REPLACE FUNCTION count_events() +RETURNS text +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + SELECT (count(*))::text AS count + FROM public.events; +END; + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events ( + payload DEFAULT count_events() +) FOR VALUES FROM (0) TO (10); + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey PRIMARY KEY (id); + +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey; + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_payload text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO public.events (id, payload) + VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; +END; + +CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id)); diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt new file mode 100644 index 000000000..c036a3542 --- /dev/null +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt @@ -0,0 +1,59 @@ +Plan: 4 to add. + +Summary by type: + functions: 2 to add + tables: 2 to add + +Functions: + + count_events + + upsert_event + +Tables: + + events + + events_pkey (constraint) + + events_grouped (policy) + ~ events (rls) + + events_leaf + +DDL to be executed: +-------------------------------------------------- + +CREATE TABLE IF NOT EXISTS events ( + id integer NOT NULL, + payload text +) PARTITION BY RANGE (id); + +ALTER TABLE events ENABLE ROW LEVEL SECURITY; + +CREATE OR REPLACE FUNCTION count_events() +RETURNS text +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + SELECT (count(*))::text AS count + FROM public.events; +END; + +CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events ( + payload DEFAULT count_events() +) FOR VALUES FROM (0) TO (10); + +ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); + +ALTER TABLE ONLY events_leaf ADD CONSTRAINT events_leaf_pkey PRIMARY KEY (id); + +ALTER INDEX events_pkey ATTACH PARTITION events_leaf_pkey; + +CREATE OR REPLACE FUNCTION upsert_event( + p_id integer, + p_payload text +) +RETURNS void +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + INSERT INTO public.events (id, payload) + VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; +END; + +CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id)); From 312a42be8d3528c80e76450668de4812f6be69bd Mon Sep 17 00:00:00 2001 From: tianzhou Date: Mon, 26 Oct 2026 22:38:27 -0700 Subject: [PATCH 4/4] fix: close remaining ordering gaps around postponed partition keys Review of #663 found three more cases where a valid schema would fail to replay or apply, plus one that is circular by construction. A SQL-standard procedure whose body depends on a postponed key was created before the batch that completes the hierarchy. Procedures now get the same hold-back as functions. The exemption that keeps a function in place when a table default or domain needs it did not follow transitive callees, so a wrapper needed by a partition default stayed while the function it calls moved later, and the wrapper then followed it. The needed set now closes over the functions a SQL body calls. A policy on another table that relies on the partitioned key was deferred correctly but bundled back into its table's file in multi-file dumps, ahead of the constraint file. Any policy the diff emits apart from its table now keeps that position under policies/, one file per table. The multi-file hoisting of relation-free function files also keeps a SQL function below the files of the functions it calls, which the wrapper case exposed. The chain fixture and the partition identity test cover the wrapper, the procedure and the cross-table policy. Co-Authored-By: Claude Fable 5.1 --- cmd/dump/partition_index_integration_test.go | 13 +- docs/cli/dump.mdx | 10 +- internal/diff/diff.go | 114 ++++++++++++++---- internal/dump/formatter.go | 90 ++++++++------ .../diff.sql | 32 ++++- .../new.sql | 36 +++++- .../plan.json | 38 +++++- .../plan.sql | 32 ++++- .../plan.txt | 47 +++++++- 9 files changed, 335 insertions(+), 77 deletions(-) diff --git a/cmd/dump/partition_index_integration_test.go b/cmd/dump/partition_index_integration_test.go index c27484d4e..342ae5abd 100644 --- a/cmd/dump/partition_index_integration_test.go +++ b/cmd/dump/partition_index_integration_test.go @@ -98,10 +98,19 @@ END; CREATE FUNCTION count_keyed_events() RETURNS text LANGUAGE sql BEGIN ATOMIC SELECT count(*)::text FROM keyed_events; END; -CREATE TABLE keyed_events_rest PARTITION OF keyed_events (note DEFAULT count_keyed_events()) DEFAULT; --- A policy expression relying on the key follows the postponed constraints. +CREATE FUNCTION count_keyed_events_text() RETURNS text LANGUAGE sql BEGIN ATOMIC + SELECT count_keyed_events(); +END; +CREATE TABLE keyed_events_rest PARTITION OF keyed_events (note DEFAULT count_keyed_events_text()) DEFAULT; +CREATE PROCEDURE upsert_keyed_event_proc(p_bucket int, p_id int) LANGUAGE sql BEGIN ATOMIC + INSERT INTO keyed_events (bucket, id) VALUES (p_bucket, p_id) ON CONFLICT ON CONSTRAINT keyed_events_pkey DO NOTHING; +END; +-- A policy expression relying on the key follows the postponed constraints, +-- on the keyed table and on a table that references it. ALTER TABLE keyed_events ENABLE ROW LEVEL SECURITY; CREATE POLICY keyed_events_grouped ON keyed_events USING (EXISTS (SELECT note FROM keyed_events GROUP BY bucket, id)); +ALTER TABLE keyed_links ENABLE ROW LEVEL SECURITY; +CREATE POLICY keyed_links_grouped ON keyed_links USING (EXISTS (SELECT note FROM keyed_events GROUP BY bucket, id)); ` for _, constraintType := range []string{"UNIQUE", "PRIMARY KEY"} { for _, multiFile := range []bool{false, true} { diff --git a/docs/cli/dump.mdx b/docs/cli/dump.mdx index a1c1f6294..b74cc9ecd 100644 --- a/docs/cli/dump.mdx +++ b/docs/cli/dump.mdx @@ -270,9 +270,13 @@ Instead, `pgschema` adds the constraint after the tables with `ALTER TABLE ONLY ... ADD CONSTRAINT`, adds each partition's copy under its original name the same way, and attaches the copies with `ALTER INDEX ... ATTACH PARTITION`. Multi-file dumps write each constraint and -its copies under `constraints/`, such as `constraints/0001_events.sql`. Row-level -security policies on such a table follow its constraints, under `policies/` in -multi-file dumps, because a policy expression may rely on the key. +its copies under `constraints/`, such as `constraints/0001_events.sql`. + +A row-level security policy that must wait for other objects, because it refers +to another new table or function or because its table's keys are added after +the tables, is created after them. Multi-file dumps keep such a policy at that +position under `policies/`, such as `policies/0001_events.sql`, rather than in +its table's file. Each individual file (e.g., `tables/users.sql`) contains the specific object definition: ```sql diff --git a/internal/diff/diff.go b/internal/diff/diff.go index edd4d4518..7a73ccbd0 100644 --- a/internal/diff/diff.go +++ b/internal/diff/diff.go @@ -2145,7 +2145,10 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto generateCreateTypesSQL(domainsWithFunctionDepsOnly, targetSchema, collector) // Create procedures (procedures may depend on tables and domains) - generateCreateProceduresSQL(d.addedProcedures, targetSchema, collector) + // A procedure whose body may depend on a postponed key (issue #653) waits + // for the batch in which that hierarchy completes, like a function. + proceduresNow, proceduresAfterSecondBatch, proceduresAfterThirdBatch := holdBackKeyDependentProcedures(d.partitionIndexes, batches, d.addedTables, d.addedProcedures) + generateCreateProceduresSQL(proceduresNow, 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, d.partitionIndexes) @@ -2153,6 +2156,7 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto // 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) generateCreateFunctionsSQL(functionsWithTableDeps, targetSchema, collector) + generateCreateProceduresSQL(proceduresAfterSecondBatch, targetSchema, collector) // Create domains whose base type is a new table's implicit row type (issue #535). // Emitted after ALL other tables so the row type exists, even if the domain also @@ -2164,6 +2168,7 @@ func (d *ddlDiff) generateCreateSQL(targetSchema string, collector *diffCollecto // Create functions that reference tables in the last table batch above. generateCreateFunctionsSQL(functionsAfterAllTables, targetSchema, collector) + generateCreateProceduresSQL(proceduresAfterThirdBatch, targetSchema, collector) // Emit COMMENT ON SEQUENCE for sequences created implicitly via CREATE TABLE (SERIAL/BIGSERIAL). // These were skipped from addedSequences but their comments must still be deployed. @@ -3076,36 +3081,38 @@ func holdBackKeyDependentFunctions(plan *partitionIndexCreationPlan, batches map if plan == nil || len(plan.roots) == 0 { return first, second, third } - completion := make(map[string]int) - lookups := make(map[string]map[string]struct{}) - for _, table := range addedTables { - root := plan.hierarchyRoot(table) - if root == "" { - continue - } - key := table.Schema + "." + table.Name - if batches[key] > completion[root] { - completion[root] = batches[key] - } - if lookups[root] == nil { - lookups[root] = make(map[string]struct{}) - } - lookups[root][strings.ToLower(key)] = struct{}{} - lookups[root][strings.ToLower(table.Name)] = struct{}{} - } - neededEarlier := func(fn *ir.Function) bool { + completion, lookups := hierarchyCompletion(plan, batches, addedTables) + pool := append(append(append([]*ir.Function{}, first...), second...), third...) + needed := make(map[*ir.Function]bool) + for _, fn := range pool { lookup := buildFunctionLookup([]*ir.Function{fn}) for _, table := range addedTables { if tableReferencesNewFunction(table, lookup) { - return true + needed[fn] = true + break } } for _, domain := range domains { - if domainReferencesNewFunction(domain, lookup) { - return true + if !needed[fn] && domainReferencesNewFunction(domain, lookup) { + needed[fn] = true + } + } + } + // A SQL function needed earlier is validated against the functions it + // calls when it is created, so those are needed earlier as well. + for changed := true; changed; { + changed = false + for fn := range needed { + if !strings.EqualFold(fn.Language, "sql") { + continue + } + for _, callee := range pool { + if !needed[callee] && referencesNewFunction(fn.Definition, fn.Schema, buildFunctionLookup([]*ir.Function{callee})) { + needed[callee] = true + changed = true + } } } - return false } var result [3][]*ir.Function for batch, fns := range [][]*ir.Function{first, second, third} { @@ -3116,7 +3123,7 @@ func holdBackKeyDependentFunctions(plan *partitionIndexCreationPlan, batches map target = completion[root] } } - if target != batch && neededEarlier(fn) { + if target != batch && needed[fn] { target = batch } result[target] = append(result[target], fn) @@ -3125,6 +3132,53 @@ func holdBackKeyDependentFunctions(plan *partitionIndexCreationPlan, batches map return result[0], result[1], result[2] } +// hierarchyCompletion returns, per hierarchy root with postponed keys, the +// table batch in which its last partition is created, and a relation lookup +// of its tables in the form bodyReferencesRelation expects. +func hierarchyCompletion(plan *partitionIndexCreationPlan, batches map[string]int, addedTables []*ir.Table) (map[string]int, map[string]map[string]struct{}) { + completion := make(map[string]int) + lookups := make(map[string]map[string]struct{}) + for _, table := range addedTables { + root := plan.hierarchyRoot(table) + if root == "" { + continue + } + key := table.Schema + "." + table.Name + if batches[key] > completion[root] { + completion[root] = batches[key] + } + if lookups[root] == nil { + lookups[root] = make(map[string]struct{}) + } + lookups[root][strings.ToLower(key)] = struct{}{} + lookups[root][strings.ToLower(table.Name)] = struct{}{} + } + return completion, lookups +} + +// holdBackKeyDependentProcedures is the procedure counterpart of +// holdBackKeyDependentFunctions. A SQL-standard procedure body is validated +// at creation, so one referencing a hierarchy that completes in a later +// table batch is created after that batch. Nothing created earlier can +// depend on a procedure, so none is needed earlier. +func holdBackKeyDependentProcedures(plan *partitionIndexCreationPlan, batches map[string]int, addedTables []*ir.Table, procedures []*ir.Procedure) ([]*ir.Procedure, []*ir.Procedure, []*ir.Procedure) { + if plan == nil || len(plan.roots) == 0 { + return procedures, nil, nil + } + completion, lookups := hierarchyCompletion(plan, batches, addedTables) + var result [3][]*ir.Procedure + for _, procedure := range procedures { + target := 0 + for root, lookup := range lookups { + if completion[root] > target && bodyReferencesRelation(procedure.Definition, lookup) { + target = completion[root] + } + } + result[target] = append(result[target], procedure) + } + return result[0], result[1], result[2] +} + func holdBackCallersAcrossBatches(first, second, third []*ir.Function) ([]*ir.Function, []*ir.Function, []*ir.Function) { for { secondLen, thirdLen := len(second), len(third) @@ -3253,6 +3307,18 @@ func RelationLookup(tables []*ir.Table, views []*ir.View) map[string]struct{} { // FunctionSignatureReferencesRelation reports whether a function's return type // or parameter types name a relation in the lookup (see RelationLookup). Such a // function must be created after that relation exists. +// FunctionLookup builds the lookup SQLFunctionCallsFunction expects. +func FunctionLookup(functions []*ir.Function) map[string]struct{} { + return buildFunctionLookup(functions) +} + +// SQLFunctionCallsFunction reports whether a SQL-language function's body +// calls one of the functions in the lookup. PostgreSQL resolves such a body +// when the function is created, so the callee must exist by then. +func SQLFunctionCallsFunction(fn *ir.Function, functions map[string]struct{}) bool { + return fn != nil && strings.EqualFold(fn.Language, "sql") && referencesNewFunction(fn.Definition, fn.Schema, functions) +} + func FunctionSignatureReferencesRelation(fn *ir.Function, relations map[string]struct{}) bool { return functionSignatureReferencesRelation(fn, relations) } diff --git a/internal/dump/formatter.go b/internal/dump/formatter.go index 30122dace..511bf0f56 100644 --- a/internal/dump/formatter.go +++ b/internal/dump/formatter.go @@ -96,30 +96,13 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er partitionIndexFiles := make(map[indexKey]fileKey) var tables []*ir.Table var views []*ir.View - // Tables whose PRIMARY KEY or UNIQUE constraints were emitted as separate - // steps after the tables (partitioned tables, issue #653), with the tables - // seen so far for walking a partition up to such a table. - keyedTables := make(map[string]bool) - tablesByKey := make(map[string]*ir.Table) - policyIndex := 0 - policyFollowsKeys := func(policy *ir.RLSPolicy) bool { - key := policy.Schema + "." + policy.Table - for key != "" { - if keyedTables[key] { - return true - } - table := tablesByKey[key] - if table == nil || table.PartitionOf == "" { - return false - } - parentSchema := table.PartitionOfSchema - if parentSchema == "" { - parentSchema = table.Schema - } - key = parentSchema + "." + table.PartitionOf - } - return false - } + // A policy the diff emits apart from its table, because it needs other + // tables, functions or postponed keys (issue #653) first, keeps that + // position in its own file instead of moving back into the table file + // ahead of what it needs. Inline policies directly follow their table's + // statements, so the previous step's file tells the two apart. + policyFiles := make(map[string]fileKey) + var prevKey fileKey for _, step := range diffs { var dir string @@ -139,15 +122,14 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er key = fileKey{dir: "constraints", name: fmt.Sprintf("%04d_%s", constraintIndex, key.name)} } - if constraint, ok := step.Source.(*ir.Constraint); ok && step.Type == diff.DiffTypeTableConstraint && - (constraint.Type == ir.ConstraintTypePrimaryKey || constraint.Type == ir.ConstraintTypeUnique) { - keyedTables[constraint.Schema+"."+constraint.Table] = true - } - if policy, ok := step.Source.(*ir.RLSPolicy); ok && step.Type == diff.DiffTypeTablePolicy && policyFollowsKeys(policy) { - // A policy created after its table's postponed keys stays after - // them instead of moving back into the table file. - policyIndex++ - key = fileKey{dir: "policies", name: fmt.Sprintf("%04d_%s", policyIndex, policy.Name)} + if policy, ok := step.Source.(*ir.RLSPolicy); ok && step.Type == diff.DiffTypeTablePolicy && len(fileOrder) > 0 && prevKey != key { + tableKey := policy.Schema + "." + policy.Table + if existing, exists := policyFiles[tableKey]; exists { + key = existing + } else { + key = fileKey{dir: "policies", name: fmt.Sprintf("%04d_%s", len(policyFiles)+1, policy.Table)} + policyFiles[tableKey] = key + } } if index, ok := step.Source.(*ir.Index); ok && index.IsPartitioned && @@ -168,11 +150,11 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er fileOrder = append(fileOrder, key) } stepsByFile[key] = append(stepsByFile[key], step) + prevKey = key switch src := step.Source.(type) { case *ir.Table: tables = append(tables, src) - tablesByKey[src.Schema+"."+src.Name] = src case *ir.View: views = append(views, src) } @@ -194,11 +176,38 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er } } relations := diff.RelationLookup(tables, views) + hoisted := make(map[fileKey]bool) + for _, key := range fileOrder[firstTable:] { + if key.dir == "functions" && !f.functionFileReferencesRelation(stepsByFile[key], relations) { + hoisted[key] = true + } + } + // A SQL function is resolved against the functions it calls when it is + // created, so its file is hoisted only if every callee's file is too. + for changed := true; changed; { + changed = false + var staying []*ir.Function + for _, key := range fileOrder[firstTable:] { + if key.dir == "functions" && !hoisted[key] { + staying = append(staying, functionsIn(stepsByFile[key])...) + } + } + lookup := diff.FunctionLookup(staying) + for key := range hoisted { + for _, fn := range functionsIn(stepsByFile[key]) { + if diff.SQLFunctionCallsFunction(fn, lookup) { + delete(hoisted, key) + changed = true + break + } + } + } + } orderedFiles := make([]fileKey, 0, len(fileOrder)) orderedFiles = append(orderedFiles, fileOrder[:firstTable]...) var rest []fileKey for _, key := range fileOrder[firstTable:] { - if key.dir == "functions" && !f.functionFileReferencesRelation(stepsByFile[key], relations) { + if hoisted[key] { orderedFiles = append(orderedFiles, key) } else { rest = append(rest, key) @@ -305,6 +314,17 @@ func (f *DumpFormatter) writeObjectFile(filePath string, diffs []diff.Diff) erro return nil } +// functionsIn returns the functions defined by the steps of a functions/ file. +func functionsIn(steps []diff.Diff) []*ir.Function { + var functions []*ir.Function + for _, step := range steps { + if fn, ok := step.Source.(*ir.Function); ok && step.Type == diff.DiffTypeFunction { + functions = append(functions, fn) + } + } + return functions +} + // functionFileReferencesRelation reports whether any function in a functions/ // file depends on a table or view existing at creation time. Such a file must // keep the diff's placement after that relation rather than being hoisted diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql index 0060ae996..2ac8add49 100644 --- a/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/diff.sql @@ -5,6 +5,13 @@ CREATE TABLE IF NOT EXISTS events ( ALTER TABLE events ENABLE ROW LEVEL SECURITY; +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +ALTER TABLE event_notes ENABLE ROW LEVEL SECURITY; + CREATE OR REPLACE FUNCTION count_events() RETURNS text LANGUAGE sql @@ -14,8 +21,16 @@ BEGIN ATOMIC FROM events; END; +CREATE OR REPLACE FUNCTION count_events_text() +RETURNS text +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + SELECT count_events() AS count_events; +END; + CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events ( - payload DEFAULT count_events() + payload DEFAULT count_events_text() ) FOR VALUES FROM (0) TO (10); ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); @@ -36,4 +51,19 @@ BEGIN ATOMIC VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; END; +CREATE OR REPLACE PROCEDURE upsert_event_proc( + IN p_id integer, + IN p_payload text +) +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO events (id, payload) + VALUES (upsert_event_proc.p_id, upsert_event_proc.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; +END; + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + +CREATE POLICY event_notes_grouped ON event_notes TO PUBLIC USING (EXISTS ( SELECT events.payload FROM events GROUP BY events.id)); + CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id)); diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql index f73f00231..2a3da46d3 100644 --- a/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/new.sql @@ -1,8 +1,8 @@ --- The partition's default calls a SQL function that reads the parent, so the --- parent, the function and the partition keep that order even though the --- parent's key is postponed until the partition exists (issue #653). A --- function that nothing else needs waits for the key, and so does the --- parent's policy, whose expression relies on the key. +-- The partition's default calls a SQL wrapper around a function that reads +-- the parent, so the parent, both functions and the partition keep that order +-- even though the parent's key is postponed until the partition exists +-- (issue #653). A function or procedure that nothing else needs waits for the +-- key, and so does any policy whose expression relies on the key. CREATE TABLE public.events ( id integer PRIMARY KEY, payload text @@ -14,8 +14,14 @@ BEGIN ATOMIC SELECT count(*)::text FROM public.events; END; +CREATE FUNCTION public.count_events_text() RETURNS text +LANGUAGE sql +BEGIN ATOMIC + SELECT public.count_events(); +END; + CREATE TABLE public.events_leaf PARTITION OF public.events ( - payload DEFAULT public.count_events() + payload DEFAULT public.count_events_text() ) FOR VALUES FROM (0) TO (10); CREATE FUNCTION public.upsert_event(p_id integer, p_payload text) RETURNS void @@ -25,7 +31,25 @@ BEGIN ATOMIC ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = EXCLUDED.payload; END; +CREATE PROCEDURE public.upsert_event_proc(p_id integer, p_payload text) +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO public.events (id, payload) VALUES (p_id, p_payload) + ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = EXCLUDED.payload; +END; + ALTER TABLE public.events ENABLE ROW LEVEL SECURITY; CREATE POLICY events_grouped ON public.events USING (EXISTS (SELECT payload FROM public.events GROUP BY id)); + +-- A policy on another table may rely on the parent's key as well. +CREATE TABLE public.event_notes ( + event_id integer NOT NULL REFERENCES public.events (id), + note text +); + +ALTER TABLE public.event_notes ENABLE ROW LEVEL SECURITY; + +CREATE POLICY event_notes_grouped ON public.event_notes + USING (EXISTS (SELECT payload FROM public.events GROUP BY id)); diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json index 859a5a778..2c0fb430c 100644 --- a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.json @@ -20,6 +20,18 @@ "operation": "alter", "path": "public.events" }, + { + "sql": "CREATE TABLE IF NOT EXISTS event_notes (\n event_id integer NOT NULL,\n note text\n);", + "type": "table", + "operation": "create", + "path": "public.event_notes" + }, + { + "sql": "ALTER TABLE event_notes ENABLE ROW LEVEL SECURITY;", + "type": "table.rls", + "operation": "alter", + "path": "public.event_notes" + }, { "sql": "CREATE OR REPLACE FUNCTION count_events()\nRETURNS text\nLANGUAGE sql\nVOLATILE\nBEGIN ATOMIC\n SELECT (count(*))::text AS count\n FROM public.events;\nEND;", "type": "function", @@ -27,7 +39,13 @@ "path": "public.count_events" }, { - "sql": "CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events (\n payload DEFAULT count_events()\n) FOR VALUES FROM (0) TO (10);", + "sql": "CREATE OR REPLACE FUNCTION count_events_text()\nRETURNS text\nLANGUAGE sql\nVOLATILE\nBEGIN ATOMIC\n SELECT public.count_events() AS count_events;\nEND;", + "type": "function", + "operation": "create", + "path": "public.count_events_text" + }, + { + "sql": "CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events (\n payload DEFAULT count_events_text()\n) FOR VALUES FROM (0) TO (10);", "type": "table", "operation": "create", "path": "public.events_leaf" @@ -56,6 +74,24 @@ "operation": "create", "path": "public.upsert_event" }, + { + "sql": "CREATE OR REPLACE PROCEDURE upsert_event_proc(\n IN p_id integer,\n IN p_payload text\n)\nLANGUAGE sql\nBEGIN ATOMIC\n INSERT INTO public.events (id, payload)\n VALUES (upsert_event_proc.p_id, upsert_event_proc.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload;\nEND;", + "type": "procedure", + "operation": "create", + "path": "public.upsert_event_proc" + }, + { + "sql": "ALTER TABLE event_notes\nADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id);", + "type": "table.constraint", + "operation": "create", + "path": "public.event_notes.event_notes_event_id_fkey" + }, + { + "sql": "CREATE POLICY event_notes_grouped ON event_notes TO PUBLIC USING (EXISTS ( SELECT events.payload FROM events GROUP BY events.id));", + "type": "table.policy", + "operation": "create", + "path": "public.event_notes.event_notes_grouped" + }, { "sql": "CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id));", "type": "table.policy", diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql index c63b0b198..e01e2add7 100644 --- a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.sql @@ -5,6 +5,13 @@ CREATE TABLE IF NOT EXISTS events ( ALTER TABLE events ENABLE ROW LEVEL SECURITY; +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +ALTER TABLE event_notes ENABLE ROW LEVEL SECURITY; + CREATE OR REPLACE FUNCTION count_events() RETURNS text LANGUAGE sql @@ -14,8 +21,16 @@ BEGIN ATOMIC FROM public.events; END; +CREATE OR REPLACE FUNCTION count_events_text() +RETURNS text +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + SELECT public.count_events() AS count_events; +END; + CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events ( - payload DEFAULT count_events() + payload DEFAULT count_events_text() ) FOR VALUES FROM (0) TO (10); ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); @@ -36,4 +51,19 @@ BEGIN ATOMIC VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; END; +CREATE OR REPLACE PROCEDURE upsert_event_proc( + IN p_id integer, + IN p_payload text +) +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO public.events (id, payload) + VALUES (upsert_event_proc.p_id, upsert_event_proc.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; +END; + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + +CREATE POLICY event_notes_grouped ON event_notes TO PUBLIC USING (EXISTS ( SELECT events.payload FROM events GROUP BY events.id)); + CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id)); diff --git a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt index c036a3542..cc07ce4f3 100644 --- a/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt +++ b/testdata/diff/create_table/issue_653_partition_default_function_chain/plan.txt @@ -1,14 +1,23 @@ -Plan: 4 to add. +Plan: 7 to add. Summary by type: - functions: 2 to add - tables: 2 to add + functions: 3 to add + procedures: 1 to add + tables: 3 to add Functions: + count_events + + count_events_text + upsert_event +Procedures: + + upsert_event_proc + Tables: + + event_notes + + event_notes_event_id_fkey (constraint) + + event_notes_grouped (policy) + ~ event_notes (rls) + events + events_pkey (constraint) + events_grouped (policy) @@ -25,6 +34,13 @@ CREATE TABLE IF NOT EXISTS events ( ALTER TABLE events ENABLE ROW LEVEL SECURITY; +CREATE TABLE IF NOT EXISTS event_notes ( + event_id integer NOT NULL, + note text +); + +ALTER TABLE event_notes ENABLE ROW LEVEL SECURITY; + CREATE OR REPLACE FUNCTION count_events() RETURNS text LANGUAGE sql @@ -34,8 +50,16 @@ BEGIN ATOMIC FROM public.events; END; +CREATE OR REPLACE FUNCTION count_events_text() +RETURNS text +LANGUAGE sql +VOLATILE +BEGIN ATOMIC + SELECT public.count_events() AS count_events; +END; + CREATE TABLE IF NOT EXISTS events_leaf PARTITION OF events ( - payload DEFAULT count_events() + payload DEFAULT count_events_text() ) FOR VALUES FROM (0) TO (10); ALTER TABLE ONLY events ADD CONSTRAINT events_pkey PRIMARY KEY (id); @@ -56,4 +80,19 @@ BEGIN ATOMIC VALUES (upsert_event.p_id, upsert_event.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; END; +CREATE OR REPLACE PROCEDURE upsert_event_proc( + IN p_id integer, + IN p_payload text +) +LANGUAGE sql +BEGIN ATOMIC + INSERT INTO public.events (id, payload) + VALUES (upsert_event_proc.p_id, upsert_event_proc.p_payload) ON CONFLICT ON CONSTRAINT events_pkey DO UPDATE SET payload = excluded.payload; +END; + +ALTER TABLE event_notes +ADD CONSTRAINT event_notes_event_id_fkey FOREIGN KEY (event_id) REFERENCES events (id); + +CREATE POLICY event_notes_grouped ON event_notes TO PUBLIC USING (EXISTS ( SELECT events.payload FROM events GROUP BY events.id)); + CREATE POLICY events_grouped ON events TO PUBLIC USING (EXISTS ( SELECT events_1.payload FROM events events_1 GROUP BY events_1.id));