At 2026-09-08 17:55:36, "Matthias van de Meent" <[email protected]> wrote: >Attached is v2 of the patchset. Changelist below: > >0001/0002: Unchanged. > >0003 (Deduplication): >* New memory context "Relation shape cache" to hold all shape-related >allocations and contexts, > This is a child context under CacheMemoryContext >* Memory context per "relation shape" in the RelShapeHash > This allows faster freeing of all associated data >* Optimized default shape entry data allocations per shape > The minimum is now down to 2 allocations, from >3. This is >primarily useful once Proxy contexts are used; Key data is still >bulk-allocated. >* Some varlena macro-related fixes. > Some SIZE/SIZE_EXHDR confusion and related issues, identified by >the sanitizer CF builds. > >0004 (Proxy context): >* Consistency checks have been adjusted, and sentinel checks have been >introduced. >* Code has been updated with aset as template for naming and flow > This should clean up David's comments. > >Name change from Proxy to anything else gets a 0-vote from me: I'd >like to avoid the churn, but if people have strong feelings about it >I'll go through the motions. > > >0005 (Apply proxy): >* Added Proxy to the new per-"index shape" contexts. > > >Question for the crowd: Most memory contexts often get a text >identifier which describes their contents in more detail when we have >many of the same name. "index shape" contexts don't have a simple >natural identifier. Whilst they do have the shape key, formatting >that into a name would be a bit of effort (and quite a bit of effort >if we want to capture the whole key), and we'd spend more bytes per >index shape. Do we want/need this identifier even with the increase >in memory usage? > > >Kind regards, > >Matthias van de Meent
>Databricks (https://www.databricks.com) Hi, Thanks for your patches. I have some review comments regarding the code below: v2-0003-Deduplicate-some-index-attributes-in-the-relcache.patch @@ -2461,6 +2656,28 @@ RelationDestroyRelation(Relation relation, bool remember_tupdesc) /* break mutual link with stats entry */ pgstat_unlink_relation(relation); + if (relation->rd_isvalid && relation->rd_indam) + { + Datum indclassDatum; + bool isnull; + oidvector *indclass; + + Assert(relation->rd_indextuple != NULL); + + indclassDatum = fastgetattr(relation->rd_indextuple, + Anum_pg_index_indclass, + GetPgIndexDescriptor(), + &isnull); + Assert(!isnull); + + indclass = (oidvector *) DatumGetPointer(indclassDatum); + + IndexSupportDeregister(indclass, + IndexRelationGetNumberOfKeyAttributes(relation), + relation->rd_indam->amsupport, + relation->rd_opcoptions); + } relcache.c: static void RelationClearRelation(Relation relation) { Assert(RelationHasReferenceCountZero(relation)); Assert(!relation->rd_isnailed); /* * Relations created in the same transaction must never be removed, see * RelationFlushRelation. */ Assert(relation->rd_createSubid == InvalidSubTransactionId); Assert(relation->rd_firstRelfilelocatorSubid == InvalidSubTransactionId); Assert(relation->rd_droppedSubid == InvalidSubTransactionId); /* first mark it as invalid */ RelationInvalidateRelation(relation); /* Remove it from the hash table */ RelationCacheDelete(relation); /* And release storage */ RelationDestroyRelation(relation, false); } The IndexSupportDeregister() cleanup inside RelationDestroyRelation() cannot be executed, since relation->rd_isvalid = false has already been set in RelationInvalidateRelation(). Entries in RelShapeCache cannot be removed. Is this inconsistent with your design? Run with the SQL below: CREATE TABLE m (a int); INSERT INTO m SELECT g FROM generate_series(1,500) g; CREATE INDEX mi ON m USING brin (a int4_bloom_ops (false_positive_rate = 0.00100000)); DROP INDEX mi; CREATE INDEX mi ON m USING brin (a int4_bloom_ops (false_positive_rate = 0.00100100)); DROP INDEX mi; CREATE INDEX mi ON m USING brin (a int4_bloom_ops (false_positive_rate = 0.00100200)); DROP INDEX mi; CREATE INDEX mi ON m USING brin (a int4_bloom_ops (false_positive_rate = 0.00100300)); DROP INDEX mi; ... When constructed in this way, the number of potential RelShapeCache entries may grow far larger than expected. Best regards, Yilin Zhang
