From bc919d95317260d7820dacf2eb4d8445b45d2915 Mon Sep 17 00:00:00 2001 From: David Geier Date: Thu, 3 Sep 2026 09:46:30 +0200 Subject: [PATCH v1 2/8] Use union --- src/backend/utils/cache/partcache.c | 25 +++- src/backend/utils/cache/relcache.c | 214 ++++++++++++++++++---------- src/include/utils/rel_internal.h | 181 +++++++++++++---------- 3 files changed, 268 insertions(+), 152 deletions(-) diff --git a/src/backend/utils/cache/partcache.c b/src/backend/utils/cache/partcache.c index a0982674884..c86efd65dd7 100644 --- a/src/backend/utils/cache/partcache.c +++ b/src/backend/utils/cache/partcache.c @@ -344,12 +344,28 @@ generate_partition_qual(Relation rel) *result = NIL; Oid parentrelid; Relation parent; + bool is_index; /* Guard against stack overflow due to overly deep partition tree */ check_stack_depth(); + /* + * rd_partcheck/rd_partcheckvalid/rd_partcheckcxt only exist for + * table-like relations; they are overlaid in a union with index-only + * fields (see RelationData in rel_internal.h). generate_partition_qual() + * is also invoked for partitioned indexes (an index partition never has + * a partition bound of its own, so this always ends up computing an + * empty qual list for them), so guard against misinterpreting unrelated + * index data, and don't cache results for indexes. + */ + if (rel->rd_rel->relkind == RELKIND_INDEX || + rel->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) + is_index = true; + else + is_index = false; + /* If we already cached the result, just return a copy */ - if (rel->rd_partcheckvalid) + if (!is_index && rel->rd_partcheckvalid) return copyObject(rel->rd_partcheck); /* @@ -397,6 +413,13 @@ generate_partition_qual(Relation rel) */ result = map_partition_varattnos(result, 1, rel, parent); + /* Never cache results for indexes; see comment above */ + if (is_index) + { + relation_close(parent, NoLock); + return result; + } + /* Assert that we're not leaking any old data during assignments below */ Assert(rel->rd_partcheckcxt == NULL); Assert(rel->rd_partcheck == NIL); diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index f475d703977..3552fd88667 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -1244,24 +1244,38 @@ retry: * * Note that RelationBuildRuleLock() relies on this being done after * extracting the relation's reloptions. - */ - if (relation->rd_rel->relhasrules) - RelationBuildRuleLock(relation); - else - { - relation->rd_rules = NULL; - relation->rd_rulescxt = NULL; - } + * + * Rules, triggers, and RLS policies only exist for table-like + * relations, never for indexes; relhasrules/relhastriggers/ + * relrowsecurity are guaranteed false for RELKIND_INDEX and + * RELKIND_PARTITIONED_INDEX. We must skip this whole block, rather + * than just relying on that, because rd_rules/rd_rulescxt/trigdesc/ + * rd_rsdesc are overlaid in a union with the index-only fields (see + * RelationData in rel_internal.h); the else-branches below would + * otherwise clobber the index access info that + * RelationInitIndexAccessInfo() just set up above. + */ + if (relation->rd_rel->relkind != RELKIND_INDEX && + relation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX) + { + if (relation->rd_rel->relhasrules) + RelationBuildRuleLock(relation); + else + { + relation->rd_rules = NULL; + relation->rd_rulescxt = NULL; + } - if (relation->rd_rel->relhastriggers) - RelationBuildTriggers(relation); - else - relation->trigdesc = NULL; + if (relation->rd_rel->relhastriggers) + RelationBuildTriggers(relation); + else + relation->trigdesc = NULL; - if (relation->rd_rel->relrowsecurity) - RelationBuildRowSecurity(relation); - else - relation->rd_rsdesc = NULL; + if (relation->rd_rel->relrowsecurity) + RelationBuildRowSecurity(relation); + else + relation->rd_rsdesc = NULL; + } /* * initialize the relation lock manager information @@ -2234,8 +2248,15 @@ RelationCloseCleanup(Relation relation) * If the relation is no longer open in this session, we can clean up any * stale partition descriptors it has. This is unlikely, so check to see * if there are child contexts before expending a call to mcxt.c. + * + * rd_pdcxt/rd_pddcxt only exist for table-like relations; they are + * overlaid in a union with index-only fields (see RelationData in + * rel_internal.h), so this must be skipped for index relations to + * avoid misinterpreting unrelated index data as a MemoryContext. */ - if (RelationHasReferenceCountZero(relation)) + if (RelationHasReferenceCountZero(relation) && + relation->rd_rel->relkind != RELKIND_INDEX && + relation->rd_rel->relkind != RELKIND_PARTITIONED_INDEX) { if (relation->rd_pdcxt != NULL && relation->rd_pdcxt->firstchild != NULL) @@ -2440,8 +2461,23 @@ RelationReloadNailed(Relation relation) static void RelationDestroyRelation(Relation relation, bool remember_tupdesc) { + bool isindex; + Assert(RelationHasReferenceCountZero(relation)); + /* + * rd_index/rd_indexcxt/etc and rd_rules/trigdesc/rd_partkey/etc are + * overlaid in a union (see RelationData in rel_internal.h), since a + * relation is never both an index and a table-like relation at once. + * Capture which set of fields is "live" for this relation before + * freeing rd_rel, so we know which branch of the union to clean up + * below; the other branch's fields are aliases of this one's and must + * not be dereferenced. + */ + isindex = relation->rd_rel && + (relation->rd_rel->relkind == RELKIND_INDEX || + relation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX); + /* * Make sure smgr and lower levels close the relation's files, if they * weren't closed already. (This was probably done by caller, but let's @@ -2475,39 +2511,45 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc) else FreeTupleDesc(relation->rd_att); } - FreeTriggerDesc(relation->trigdesc); - list_free_deep(relation->rd_fkeylist); list_free(relation->rd_indexlist); list_free(relation->rd_statlist); - bms_free(relation->rd_keyattr); - bms_free(relation->rd_pkattr); - bms_free(relation->rd_idattr); - bms_free(relation->rd_hotblockingattr); - bms_free(relation->rd_summarizedattr); - if (relation->rd_pubdesc) - pfree(relation->rd_pubdesc); if (relation->rd_options) pfree(relation->rd_options); - if (relation->rd_indextuple) - pfree(relation->rd_indextuple); if (relation->rd_amcache) pfree(relation->rd_amcache); - if (relation->rd_fdwroutine) - pfree(relation->rd_fdwroutine); - if (relation->rd_indexcxt) - MemoryContextDelete(relation->rd_indexcxt); - if (relation->rd_rulescxt) - MemoryContextDelete(relation->rd_rulescxt); - if (relation->rd_rsdesc) - MemoryContextDelete(relation->rd_rsdesc->rscxt); - if (relation->rd_partkeycxt) - MemoryContextDelete(relation->rd_partkeycxt); - if (relation->rd_pdcxt) - MemoryContextDelete(relation->rd_pdcxt); - if (relation->rd_pddcxt) - MemoryContextDelete(relation->rd_pddcxt); - if (relation->rd_partcheckcxt) - MemoryContextDelete(relation->rd_partcheckcxt); + if (!isindex) + { + FreeTriggerDesc(relation->trigdesc); + list_free_deep(relation->rd_fkeylist); + bms_free(relation->rd_keyattr); + bms_free(relation->rd_pkattr); + bms_free(relation->rd_idattr); + bms_free(relation->rd_hotblockingattr); + bms_free(relation->rd_summarizedattr); + if (relation->rd_pubdesc) + pfree(relation->rd_pubdesc); + if (relation->rd_fdwroutine) + pfree(relation->rd_fdwroutine); + if (relation->rd_rulescxt) + MemoryContextDelete(relation->rd_rulescxt); + if (relation->rd_rsdesc) + MemoryContextDelete(relation->rd_rsdesc->rscxt); + if (relation->rd_partkeycxt) + MemoryContextDelete(relation->rd_partkeycxt); + if (relation->rd_pdcxt) + MemoryContextDelete(relation->rd_pdcxt); + if (relation->rd_pddcxt) + MemoryContextDelete(relation->rd_pddcxt); + if (relation->rd_partcheckcxt) + MemoryContextDelete(relation->rd_partcheckcxt); + } + else + { + if (relation->rd_indextuple) + pfree(relation->rd_indextuple); + if (relation->rd_indexcxt) + MemoryContextDelete(relation->rd_indexcxt); + } pfree(relation); } @@ -2699,10 +2741,29 @@ RelationRebuildRelation(Relation relation) Assert(relation->rd_rel->relkind == newrel->rd_rel->relkind); keep_tupdesc = equalTupleDescs(relation->rd_att, newrel->rd_att); - keep_rules = equalRuleLocks(relation->rd_rules, newrel->rd_rules); - keep_policies = equalRSDesc(relation->rd_rsdesc, newrel->rd_rsdesc); - /* partkey is immutable once set up, so we can always keep it */ - keep_partkey = (relation->rd_partkey != NULL); + + /* + * rd_rules/rd_rsdesc/rd_partkey only exist for table-like + * relations; they are overlaid in a union with index-only fields + * (see RelationData in rel_internal.h). We normally only reach + * this generic rebuild path for indexes whose access info hasn't + * been initialized yet (see above), but guard explicitly anyway to + * avoid misinterpreting unrelated index data. + */ + if (relation->rd_rel->relkind == RELKIND_INDEX || + relation->rd_rel->relkind == RELKIND_PARTITIONED_INDEX) + { + keep_rules = false; + keep_policies = false; + keep_partkey = false; + } + else + { + keep_rules = equalRuleLocks(relation->rd_rules, newrel->rd_rules); + keep_policies = equalRSDesc(relation->rd_rsdesc, newrel->rd_rsdesc); + /* partkey is immutable once set up, so we can always keep it */ + keep_partkey = (relation->rd_partkey != NULL); + } /* * Perform swapping of the relcache entry contents. Within this @@ -6482,27 +6543,38 @@ load_relcache_init_file(bool shared) * be a big performance hit since few system catalogs have such. Ditto * for RLS policy data, partition info, index expressions, predicates, * exclusion info, and FDW info. + * + * These fields are overlaid in a union with the index-only fields + * that were just set up above (see RelationData in rel_internal.h), + * so this must be skipped for index relations to avoid clobbering + * that data. */ - rel->rd_rules = NULL; - rel->rd_rulescxt = NULL; - rel->trigdesc = NULL; - rel->rd_rsdesc = NULL; - rel->rd_partkey = NULL; - rel->rd_partkeycxt = NULL; - rel->rd_partdesc = NULL; - rel->rd_partdesc_nodetached = NULL; - rel->rd_partdesc_nodetached_xmin = InvalidTransactionId; - rel->rd_pdcxt = NULL; - rel->rd_pddcxt = NULL; - rel->rd_partcheck = NIL; - rel->rd_partcheckvalid = false; - rel->rd_partcheckcxt = NULL; - rel->rd_indexprs = NIL; - rel->rd_indpred = NIL; - rel->rd_exclops = NULL; - rel->rd_exclprocs = NULL; - rel->rd_exclstrats = NULL; - rel->rd_fdwroutine = NULL; + if (rel->rd_rel->relkind != RELKIND_INDEX) + { + rel->rd_rules = NULL; + rel->rd_rulescxt = NULL; + rel->trigdesc = NULL; + rel->rd_rsdesc = NULL; + rel->rd_partkey = NULL; + rel->rd_partkeycxt = NULL; + rel->rd_partdesc = NULL; + rel->rd_partdesc_nodetached = NULL; + rel->rd_partdesc_nodetached_xmin = InvalidTransactionId; + rel->rd_pdcxt = NULL; + rel->rd_pddcxt = NULL; + rel->rd_partcheck = NIL; + rel->rd_partcheckvalid = false; + rel->rd_partcheckcxt = NULL; + rel->rd_keyattr = NULL; + rel->rd_pkattr = NULL; + rel->rd_idattr = NULL; + rel->rd_hotblockingattr = NULL; + rel->rd_summarizedattr = NULL; + rel->rd_pubdesc = NULL; + rel->rd_fdwroutine = NULL; + rel->rd_fkeyvalid = false; + rel->rd_fkeylist = NIL; + } /* * Reset transient-state fields in the relcache entry @@ -6517,14 +6589,8 @@ load_relcache_init_file(bool shared) rel->rd_pkindex = InvalidOid; rel->rd_replidindex = InvalidOid; rel->rd_attrsvalid = false; - rel->rd_keyattr = NULL; - rel->rd_pkattr = NULL; - rel->rd_idattr = NULL; - rel->rd_pubdesc = NULL; rel->rd_statvalid = false; rel->rd_statlist = NIL; - rel->rd_fkeyvalid = false; - rel->rd_fkeylist = NIL; rel->rd_createSubid = InvalidSubTransactionId; rel->rd_newRelfilelocatorSubid = InvalidSubTransactionId; rel->rd_firstRelfilelocatorSubid = InvalidSubTransactionId; diff --git a/src/include/utils/rel_internal.h b/src/include/utils/rel_internal.h index 675252a641e..0f2bdaba808 100644 --- a/src/include/utils/rel_internal.h +++ b/src/include/utils/rel_internal.h @@ -98,41 +98,115 @@ typedef struct RelationData TupleDesc rd_att; /* tuple descriptor */ Oid rd_id; /* relation's object id */ LockInfoData rd_lockInfo; /* lock mgr's info for locking relation */ - RuleLock *rd_rules; /* rewrite rules */ - MemoryContext rd_rulescxt; /* private memory cxt for rd_rules, if any */ - TriggerDesc *trigdesc; /* Trigger info, or NULL if rel has none */ - /* use "struct" here to avoid needing to include rowsecurity.h: */ - struct RowSecurityDesc *rd_rsdesc; /* row security policies, or NULL */ - /* data managed by RelationGetFKeyList: */ - List *rd_fkeylist; /* list of ForeignKeyCacheInfo (see below) */ - bool rd_fkeyvalid; /* true if list has been computed */ + /* + * The following two blocks of fields are mutually exclusive: a + * relation is never both an index and a non-index (table-like) + * relation at the same time. Overlapping them in a union avoids + * paying for both sets of fields in every relcache entry, which adds + * up across installations with large numbers of tables and indexes. + * Anonymous struct/union members are used so that every field below + * remains directly accessible as "relation->rd_xxx", exactly as if it + * were not inside a union. + * + * NOTE: RelationDestroyRelation() in relcache.c must only clean up the + * branch of this union that matches the relation's actual relkind; the + * fields of the other branch alias this one's memory and must not be + * dereferenced. + */ + union + { + /* fields used only for a non-index (table-like) relation */ + struct + { + RuleLock *rd_rules; /* rewrite rules */ + MemoryContext rd_rulescxt; /* private memory cxt for rd_rules, if any */ + TriggerDesc *trigdesc; /* Trigger info, or NULL if rel has none */ + /* use "struct" here to avoid needing to include rowsecurity.h: */ + struct RowSecurityDesc *rd_rsdesc; /* row security policies, or NULL */ - /* data managed by RelationGetPartitionKey: */ - PartitionKey rd_partkey; /* partition key, or NULL */ - MemoryContext rd_partkeycxt; /* private context for rd_partkey, if any */ + /* data managed by RelationGetFKeyList: */ + List *rd_fkeylist; /* list of ForeignKeyCacheInfo (see below) */ + bool rd_fkeyvalid; /* true if list has been computed */ - /* data managed by RelationGetPartitionDesc: */ - PartitionDesc rd_partdesc; /* partition descriptor, or NULL */ - MemoryContext rd_pdcxt; /* private context for rd_partdesc, if any */ + /* data managed by RelationGetPartitionKey: */ + PartitionKey rd_partkey; /* partition key, or NULL */ + MemoryContext rd_partkeycxt; /* private context for rd_partkey, if any */ - /* Same as above, for partdescs that omit detached partitions */ - PartitionDesc rd_partdesc_nodetached; /* partdesc w/o detached parts */ - MemoryContext rd_pddcxt; /* for rd_partdesc_nodetached, if any */ + /* data managed by RelationGetPartitionDesc: */ + PartitionDesc rd_partdesc; /* partition descriptor, or NULL */ + MemoryContext rd_pdcxt; /* private context for rd_partdesc, if any */ - /* - * pg_inherits.xmin of the partition that was excluded in - * rd_partdesc_nodetached. This informs a future user of that partdesc: - * if this value is not in progress for the active snapshot, then the - * partdesc can be used, otherwise they have to build a new one. (This - * matches what find_inheritance_children_extended would do). - */ - TransactionId rd_partdesc_nodetached_xmin; + /* Same as above, for partdescs that omit detached partitions */ + PartitionDesc rd_partdesc_nodetached; /* partdesc w/o detached parts */ + MemoryContext rd_pddcxt; /* for rd_partdesc_nodetached, if any */ + + /* + * pg_inherits.xmin of the partition that was excluded in + * rd_partdesc_nodetached. This informs a future user of that partdesc: + * if this value is not in progress for the active snapshot, then the + * partdesc can be used, otherwise they have to build a new one. (This + * matches what find_inheritance_children_extended would do). + */ + TransactionId rd_partdesc_nodetached_xmin; + + /* data managed by RelationGetPartitionQual: */ + List *rd_partcheck; /* partition CHECK quals */ + bool rd_partcheckvalid; /* true if list has been computed */ + MemoryContext rd_partcheckcxt; /* private cxt for rd_partcheck, if any */ + + /* data managed by RelationGetIndexAttrBitmap: */ + Bitmapset *rd_keyattr; /* cols that can be ref'd by foreign keys */ + Bitmapset *rd_pkattr; /* cols included in primary key */ + Bitmapset *rd_idattr; /* included in replica identity index */ + Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */ + Bitmapset *rd_summarizedattr; /* cols indexed by summarizing indexes */ + + PublicationDesc *rd_pubdesc; /* publication descriptor, or NULL */ + + /* + * foreign-table support + * + * rd_fdwroutine must point to a single memory chunk palloc'd in + * CacheMemoryContext. It will be freed and reset to NULL on a + * relcache reset. + */ + /* use "struct" here to avoid needing to include fdwapi.h: */ + struct FdwRoutine *rd_fdwroutine; /* cached function pointers, or NULL */ + }; + + /* fields used only for an index relation */ + struct + { + Form_pg_index rd_index; /* pg_index tuple describing this index */ + /* use "struct" here to avoid needing to include htup.h: */ + struct HeapTupleData *rd_indextuple; /* all of pg_index tuple */ - /* data managed by RelationGetPartitionQual: */ - List *rd_partcheck; /* partition CHECK quals */ - bool rd_partcheckvalid; /* true if list has been computed */ - MemoryContext rd_partcheckcxt; /* private cxt for rd_partcheck, if any */ + /* + * index access support info (used only for an index relation) + * + * Note: only default support procs for each opclass are cached, namely + * those with lefttype and righttype equal to the opclass's opcintype. The + * arrays are indexed by support function number, which is a sufficient + * identifier given that restriction. + */ + MemoryContext rd_indexcxt; /* private memory cxt for this stuff */ + /* use "struct" here to avoid needing to include amapi.h: */ + const struct IndexAmRoutine *rd_indam; /* index AM's API struct */ + Oid *rd_opfamily; /* OIDs of op families for each index col */ + Oid *rd_opcintype; /* OIDs of opclass declared input data types */ + RegProcedure *rd_support; /* OIDs of support procedures */ + struct FmgrInfo *rd_supportinfo; /* lookup info for support procedures */ + int16 *rd_indoption; /* per-column AM-specific flags */ + List *rd_indexprs; /* index expression trees, if any */ + List *rd_indpred; /* index predicate tree, if any */ + Oid *rd_exclops; /* OIDs of exclusion operators, if any */ + Oid *rd_exclprocs; /* OIDs of exclusion ops' procs, if any */ + uint16 *rd_exclstrats; /* exclusion ops' strategy numbers, if any */ + Oid *rd_indcollation; /* OIDs of index collations */ + bytea **rd_opcoptions; /* parsed opclass-specific options */ + }; + }; /* data managed by RelationGetIndexList: */ List *rd_indexlist; /* list of OIDs of indexes on relation */ @@ -145,13 +219,6 @@ typedef struct RelationData /* data managed by RelationGetIndexAttrBitmap: */ bool rd_attrsvalid; /* are bitmaps of attrs valid? */ - Bitmapset *rd_keyattr; /* cols that can be ref'd by foreign keys */ - Bitmapset *rd_pkattr; /* cols included in primary key */ - Bitmapset *rd_idattr; /* included in replica identity index */ - Bitmapset *rd_hotblockingattr; /* cols blocking HOT update */ - Bitmapset *rd_summarizedattr; /* cols indexed by summarizing indexes */ - - PublicationDesc *rd_pubdesc; /* publication descriptor, or NULL */ /* * rd_options is set whenever rd_rel is loaded into the relcache entry. @@ -174,35 +241,6 @@ typedef struct RelationData */ const struct TableAmRoutine *rd_tableam; - /* These are non-NULL only for an index relation: */ - Form_pg_index rd_index; /* pg_index tuple describing this index */ - /* use "struct" here to avoid needing to include htup.h: */ - struct HeapTupleData *rd_indextuple; /* all of pg_index tuple */ - - /* - * index access support info (used only for an index relation) - * - * Note: only default support procs for each opclass are cached, namely - * those with lefttype and righttype equal to the opclass's opcintype. The - * arrays are indexed by support function number, which is a sufficient - * identifier given that restriction. - */ - MemoryContext rd_indexcxt; /* private memory cxt for this stuff */ - /* use "struct" here to avoid needing to include amapi.h: */ - const struct IndexAmRoutine *rd_indam; /* index AM's API struct */ - Oid *rd_opfamily; /* OIDs of op families for each index col */ - Oid *rd_opcintype; /* OIDs of opclass declared input data types */ - RegProcedure *rd_support; /* OIDs of support procedures */ - struct FmgrInfo *rd_supportinfo; /* lookup info for support procedures */ - int16 *rd_indoption; /* per-column AM-specific flags */ - List *rd_indexprs; /* index expression trees, if any */ - List *rd_indpred; /* index predicate tree, if any */ - Oid *rd_exclops; /* OIDs of exclusion operators, if any */ - Oid *rd_exclprocs; /* OIDs of exclusion ops' procs, if any */ - uint16 *rd_exclstrats; /* exclusion ops' strategy numbers, if any */ - Oid *rd_indcollation; /* OIDs of index collations */ - bytea **rd_opcoptions; /* parsed opclass-specific options */ - /* * rd_amcache is available for index and table AMs to cache private data * about the relation. This must be just a cache since it may get reset @@ -214,17 +252,6 @@ typedef struct RelationData */ void *rd_amcache; /* available for use by index/table AM */ - /* - * foreign-table support - * - * rd_fdwroutine must point to a single memory chunk palloc'd in - * CacheMemoryContext. It will be freed and reset to NULL on a relcache - * reset. - */ - - /* use "struct" here to avoid needing to include fdwapi.h: */ - struct FdwRoutine *rd_fdwroutine; /* cached function pointers, or NULL */ - /* * Hack for CLUSTER, rewriting ALTER TABLE, etc: when writing a new * version of a table, we need to make any toast pointers inserted into it @@ -241,4 +268,4 @@ typedef struct RelationData struct PgStat_RelationStatus *pgstat_info; /* statistics collection area */ } RelationData; -#endif /* REL_INTERNAL_H */ \ No newline at end of file +#endif /* REL_INTERNAL_H */ -- 2.51.0