fix: keep inherited constraint index names when dumping partitioned tables (#653)

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")

cmd/dump/partition_index_integration_test.go

@@ -78,6 +78,39 @@ 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));
+-- 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;
+-- 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 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} {

docs/cli/dump.mdx

@@ -263,6 +263,21 @@ 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`.
+
+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
 --

internal/diff/diff.go

@@ -2020,12 +2020,25 @@ 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))
+	for _, table := range d.addedTables {
+		key := table.Schema + "." + table.Name
+		batches[key] = tableBatch(table)
+		switch batches[key] {
+		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)
 		}
 	}
@@ -2112,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)
@@ -2127,14 +2145,18 @@ 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)
 
 	// 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
@@ -2146,13 +2168,18 @@ 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.
 	for _, seq := range d.addedSerialSeqComments {
 		generateSequenceComment(seq, targetSchema, DiffOperationCreate, collector)
 	}
 
+	// 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)
 
 	// Add deferred foreign key constraints from ALL batches AFTER all tables are created
@@ -3041,6 +3068,117 @@ 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, 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) {
+				needed[fn] = true
+				break
+			}
+		}
+		for _, domain := range domains {
+			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
+				}
+			}
+		}
+	}
+	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 && needed[fn] {
+				target = batch
+			}
+			result[target] = append(result[target], fn)
+		}
+	}
+	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)
@@ -3169,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)
 }