From bd14b97be91fac0fbe290e1cc50a012554fb807d Mon Sep 17 00:00:00 2001 From: Ayush Tiwari Date: Sat, 18 Jul 2026 00:00:00 +0530 Subject: [PATCH v2] Cache a partition index's ancestor list in the relcache get_partition_ancestors() scans pg_inherits once per hierarchy level on every call. In the INSERT ... ON CONFLICT arbiter-mapping loop in ExecInitPartitionInfo() this runs once per leaf-partition index; the existing XXX comment there flagged it as slow. Cache the full partition-index ancestor list on the index's relcache entry, computed lazily by get_partition_index_ancestors(). Following Alvaro's suggestion, the list is stored as a single OID for the common single-ancestor case, or as a palloc'd array in CacheMemoryContext for deeper hierarchies (rd_partancestorcount / rd_partancestors). It is reset on a full relcache rebuild and on the minimal index reload, and the array is freed at those points and at relcache destruction. Because the cached list depends on links owned by ancestors, re-parenting an intermediate partitioned index would otherwise leave a descendant leaf index's cache stale: changing a pg_inherits row emits no relcache invalidation, and IndexSetParentIndex() only flips the re-parented index's own pg_class row. IndexSetParentIndex() therefore now invalidates the whole descendant-index subtree via find_all_inheritors(). Re-parenting an index is a rare operation, so the extra invalidation messages are cheap. Add a regression test that re-parents an intermediate partitioned index in a three-level hierarchy and checks that the leaf index's arbiter resolution still succeeds; with a stale cache it fails with "invalid arbiter index list". --- src/backend/catalog/partition.c | 86 +++++++++++++++++++ src/backend/commands/indexcmds.c | 17 ++++ src/backend/executor/execPartition.c | 11 ++- src/backend/utils/cache/relcache.c | 10 +++ src/include/catalog/partition.h | 1 + src/include/utils/rel.h | 9 ++ src/test/regress/expected/insert_conflict.out | 52 +++++++++++ src/test/regress/sql/insert_conflict.sql | 49 +++++++++++ 8 files changed, 229 insertions(+), 6 deletions(-) diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c index 28f3cade6ff..fea01dd9b1f 100644 --- a/src/backend/catalog/partition.c +++ b/src/backend/catalog/partition.c @@ -27,6 +27,7 @@ #include "optimizer/optimizer.h" #include "rewrite/rewriteManip.h" #include "utils/fmgroids.h" +#include "utils/memutils.h" #include "utils/partcache.h" #include "utils/rel.h" #include "utils/syscache.h" @@ -35,6 +36,7 @@ static Oid get_partition_parent_worker(Relation inhRel, Oid relid, bool *detach_pending); static void get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors); +static void RelationCachePartitionIndexAncestors(Relation indexRel); /* * get_partition_parent @@ -167,6 +169,90 @@ get_partition_ancestors_worker(Relation inhRel, Oid relid, List **ancestors) get_partition_ancestors_worker(inhRel, parentOid, ancestors); } +/* + * get_partition_index_ancestors + * Return the OIDs of the partition-index ancestors of 'indexRel', from + * its immediate parent up to the topmost partitioned index, or NIL if + * 'indexRel' is not a partition. + * + * The list is cached on the relcache entry (as a single OID or a + * CacheMemoryContext array) and recomputed only after an invalidation. + * For that to be safe, re-parenting an index must invalidate its whole + * descendant-index subtree, which IndexSetParentIndex() arranges. + * + * The caller must have 'indexRel' open and locked. A freshly allocated + * list is returned, so the caller may free it. + */ +List * +get_partition_index_ancestors(Relation indexRel) +{ + List *result = NIL; + + Assert(indexRel->rd_rel->relkind == RELKIND_INDEX); + + /* Compute and cache the ancestor list on first use since (re)build. */ + if (indexRel->rd_partancestorcount == -1) + RelationCachePartitionIndexAncestors(indexRel); + + /* Rebuild a list from the cached representation. */ + if (indexRel->rd_partancestorcount == 1) + result = list_make1_oid(indexRel->rd_partancestors.single); + else + { + for (int i = 0; i < indexRel->rd_partancestorcount; i++) + result = lappend_oid(result, indexRel->rd_partancestors.array[i]); + } + + return result; +} + +/* + * RelationCachePartitionIndexAncestors + * Compute and store the partition-index ancestor list of 'indexRel' on + * its relcache entry. + * + * Stored as a single OID for one ancestor, or a palloc'd CacheMemoryContext + * array for several; rd_partancestorcount holds the count (0 when 'indexRel' + * is not a partition). Reset to -1, and the array freed, whenever the + * relcache entry is rebuilt or reloaded. + */ +static void +RelationCachePartitionIndexAncestors(Relation indexRel) +{ + List *ancestors; + int count; + + if (!indexRel->rd_rel->relispartition) + { + indexRel->rd_partancestorcount = 0; + return; + } + + ancestors = get_partition_ancestors(RelationGetRelid(indexRel)); + count = list_length(ancestors); + + if (count == 1) + indexRel->rd_partancestors.single = linitial_oid(ancestors); + else if (count > 1) + { + MemoryContext oldcxt; + Oid *array; + ListCell *lc; + int i = 0; + + oldcxt = MemoryContextSwitchTo(CacheMemoryContext); + array = (Oid *) palloc(count * sizeof(Oid)); + MemoryContextSwitchTo(oldcxt); + + foreach(lc, ancestors) + array[i++] = lfirst_oid(lc); + indexRel->rd_partancestors.array = array; + } + + indexRel->rd_partancestorcount = count; + list_free(ancestors); +} + /* * index_get_partition * Return the OID of index of the given partition that is a child diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 713bb5d10f1..a1150f92ac9 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -4723,6 +4723,23 @@ IndexSetParentIndex(Relation partitionIdx, Oid parentOid) /* make our updates visible */ CommandCounterIncrement(); } + + /* + * Any indexes attached below partitionIdx have cached partition-ancestor + * lists (see get_partition_index_ancestors) that include the link we + * just changed, so invalidate the whole descendant-index subtree. + * Re-parenting an index is rare, so the extra invalidation messages + * are cheap. + */ + { + List *descendants; + ListCell *lc; + + descendants = find_all_inheritors(partRelid, NoLock, NULL); + foreach(lc, descendants) + CacheInvalidateRelcacheByRelid(lfirst_oid(lc)); + list_free(descendants); + } } /* diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index 33ec5bfde4c..f1be6ca778d 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -786,14 +786,13 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, * This will have the effect of also treating that second * index as arbiter. * - * XXX get_partition_ancestors scans pg_inherits, which is not - * only slow, but also means the catalog snapshot can get - * invalidated each time through the loop (cf. - * GetNonHistoricCatalogSnapshot). Consider a syscache or - * some other way to cache? + * Use the already-open index relation so its ancestor list + * can be cached on the relcache entry, avoiding repeated + * pg_inherits scans in this loop. */ indexoid = RelationGetRelid(leaf_part_rri->ri_IndexRelationDescs[listidx]); - ancestors = get_partition_ancestors(indexoid); + ancestors = + get_partition_index_ancestors(leaf_part_rri->ri_IndexRelationDescs[listidx]); INJECTION_POINT("exec-init-partition-after-get-partition-ancestors", NULL); if (ancestors != NIL && diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 19c4ff6e75e..933c8318c24 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1218,6 +1218,7 @@ retry: relation->rd_partcheck = NIL; relation->rd_partcheckvalid = false; relation->rd_partcheckcxt = NULL; + relation->rd_partancestorcount = -1; /* * initialize access method information @@ -2318,6 +2319,9 @@ RelationReloadIndexInfo(Relation relation) RelationGetRelid(relation)); relp = (Form_pg_class) GETSTRUCT(pg_class_tuple); memcpy(relation->rd_rel, relp, CLASS_TUPLE_SIZE); + if (relation->rd_partancestorcount > 1) + pfree(relation->rd_partancestors.array); + relation->rd_partancestorcount = -1; /* Reload reloptions in case they changed */ if (relation->rd_options) pfree(relation->rd_options); @@ -2510,6 +2514,8 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc) MemoryContextDelete(relation->rd_pddcxt); if (relation->rd_partcheckcxt) MemoryContextDelete(relation->rd_partcheckcxt); + if (relation->rd_partancestorcount > 1) + pfree(relation->rd_partancestors.array); pfree(relation); } @@ -3599,6 +3605,9 @@ RelationBuildLocalRelation(const char *relname, rel->rd_firstRelfilelocatorSubid = InvalidSubTransactionId; rel->rd_droppedSubid = InvalidSubTransactionId; + /* the partition-index ancestor cache is computed lazily */ + rel->rd_partancestorcount = -1; + /* * create a new tuple descriptor from the one passed in. We do this * partly to copy it into the cache context, and partly because the new @@ -6500,6 +6509,7 @@ load_relcache_init_file(bool shared) rel->rd_partcheck = NIL; rel->rd_partcheckvalid = false; rel->rd_partcheckcxt = NULL; + rel->rd_partancestorcount = -1; rel->rd_indexprs = NIL; rel->rd_indpred = NIL; rel->rd_exclops = NULL; diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h index a93cf081dd2..6b7064b8ecf 100644 --- a/src/include/catalog/partition.h +++ b/src/include/catalog/partition.h @@ -21,6 +21,7 @@ extern Oid get_partition_parent(Oid relid, bool even_if_detached); extern List *get_partition_ancestors(Oid relid); +extern List *get_partition_index_ancestors(Relation indexRel); extern Oid index_get_partition(Relation partition, Oid indexId); extern List *map_partition_varattnos(List *expr, int fromrel_varno, Relation to_rel, Relation from_rel); diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index 89c159b133f..3a3f52fe9f6 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -148,6 +148,15 @@ typedef struct RelationData bool rd_partcheckvalid; /* true if list has been computed */ MemoryContext rd_partcheckcxt; /* private cxt for rd_partcheck, if any */ + /* data managed by get_partition_index_ancestors: */ + int rd_partancestorcount; /* # of cached partition-index + * ancestors, or -1 if not computed */ + union + { + Oid single; /* sole ancestor, when count == 1 */ + Oid *array; /* count entries, when count > 1 */ + } rd_partancestors; + /* data managed by RelationGetIndexList: */ List *rd_indexlist; /* list of OIDs of indexes on relation */ Oid rd_pkindex; /* OID of (deferrable?) primary key, if any */ diff --git a/src/test/regress/expected/insert_conflict.out b/src/test/regress/expected/insert_conflict.out index 34e2e7ee355..6f2dd48f413 100644 --- a/src/test/regress/expected/insert_conflict.out +++ b/src/test/regress/expected/insert_conflict.out @@ -1042,6 +1042,58 @@ insert into parted_conflict_test (a, b) values (3, 'l') on conflict (a) do selec (1 row) drop table parted_conflict_test; +-- check that a cached index parent is invalidated when its partition is +-- detached and attached to another partitioned table +create table reparent_conflict_a (i int primary key) partition by range (i); +create table reparent_conflict_b (i int primary key) partition by range (i); +create table reparent_conflict_child partition of reparent_conflict_a + for values from (0) to (10); +insert into reparent_conflict_a values (1) + on conflict (i) do update set i = excluded.i; +begin; +alter table reparent_conflict_a detach partition reparent_conflict_child; +alter table reparent_conflict_b attach partition reparent_conflict_child + for values from (0) to (10); +insert into reparent_conflict_b values (1) + on conflict (i) do update set i = excluded.i; +commit; +-- check the reverse invalidation when reparenting is rolled back +begin; +savepoint reparent_conflict_savepoint; +alter table reparent_conflict_b detach partition reparent_conflict_child; +alter table reparent_conflict_a attach partition reparent_conflict_child + for values from (0) to (10); +insert into reparent_conflict_a values (1) + on conflict (i) do update set i = excluded.i; +rollback to reparent_conflict_savepoint; +insert into reparent_conflict_b values (1) + on conflict (i) do update set i = excluded.i; +commit; +drop table reparent_conflict_a, reparent_conflict_b; +-- A leaf index caches its *full* ancestor list; re-parenting an +-- intermediate partitioned index must invalidate the leaf, else arbiter +-- resolution below uses a stale ancestor and fails with "invalid arbiter +-- index list". +create table reparent3_a (i int primary key) partition by range (i); +create table reparent3_mid partition of reparent3_a + for values from (0) to (100) partition by range (i); +create table reparent3_leaf partition of reparent3_mid + for values from (0) to (10); +create table reparent3_b (i int primary key) partition by range (i); +insert into reparent3_a values (1) on conflict (i) do nothing; +alter table reparent3_a detach partition reparent3_mid; +alter table reparent3_b attach partition reparent3_mid + for values from (0) to (100); +insert into reparent3_b values (1) on conflict (i) do update set i = excluded.i; +insert into reparent3_b values (2) on conflict (i) do update set i = excluded.i; +select * from reparent3_b order by i; + i +--- + 1 + 2 +(2 rows) + +drop table reparent3_a, reparent3_b; -- test behavior of inserting a conflicting tuple into an intermediate -- partitioning level create table parted_conflict (a int primary key, b text) partition by range (a); diff --git a/src/test/regress/sql/insert_conflict.sql b/src/test/regress/sql/insert_conflict.sql index a5a84d1d4b8..c4a104d0343 100644 --- a/src/test/regress/sql/insert_conflict.sql +++ b/src/test/regress/sql/insert_conflict.sql @@ -596,6 +596,55 @@ insert into parted_conflict_test (a, b) values (3, 'l') on conflict (a) do selec drop table parted_conflict_test; +-- check that a cached index parent is invalidated when its partition is +-- detached and attached to another partitioned table +create table reparent_conflict_a (i int primary key) partition by range (i); +create table reparent_conflict_b (i int primary key) partition by range (i); +create table reparent_conflict_child partition of reparent_conflict_a + for values from (0) to (10); +insert into reparent_conflict_a values (1) + on conflict (i) do update set i = excluded.i; +begin; +alter table reparent_conflict_a detach partition reparent_conflict_child; +alter table reparent_conflict_b attach partition reparent_conflict_child + for values from (0) to (10); +insert into reparent_conflict_b values (1) + on conflict (i) do update set i = excluded.i; +commit; + +-- check the reverse invalidation when reparenting is rolled back +begin; +savepoint reparent_conflict_savepoint; +alter table reparent_conflict_b detach partition reparent_conflict_child; +alter table reparent_conflict_a attach partition reparent_conflict_child + for values from (0) to (10); +insert into reparent_conflict_a values (1) + on conflict (i) do update set i = excluded.i; +rollback to reparent_conflict_savepoint; +insert into reparent_conflict_b values (1) + on conflict (i) do update set i = excluded.i; +commit; +drop table reparent_conflict_a, reparent_conflict_b; + +-- A leaf index caches its *full* ancestor list; re-parenting an +-- intermediate partitioned index must invalidate the leaf, else arbiter +-- resolution below uses a stale ancestor and fails with "invalid arbiter +-- index list". +create table reparent3_a (i int primary key) partition by range (i); +create table reparent3_mid partition of reparent3_a + for values from (0) to (100) partition by range (i); +create table reparent3_leaf partition of reparent3_mid + for values from (0) to (10); +create table reparent3_b (i int primary key) partition by range (i); +insert into reparent3_a values (1) on conflict (i) do nothing; +alter table reparent3_a detach partition reparent3_mid; +alter table reparent3_b attach partition reparent3_mid + for values from (0) to (100); +insert into reparent3_b values (1) on conflict (i) do update set i = excluded.i; +insert into reparent3_b values (2) on conflict (i) do update set i = excluded.i; +select * from reparent3_b order by i; +drop table reparent3_a, reparent3_b; + -- test behavior of inserting a conflicting tuple into an intermediate -- partitioning level create table parted_conflict (a int primary key, b text) partition by range (a); -- 2.43.0