On Tue, Mar 24, 2026, at 3:44 PM, Nathan Bossart wrote: > On Tue, Mar 24, 2026 at 02:02:07PM -0400, Greg Burd wrote: >> On Mon, Mar 23, 2026, at 2:39 PM, Nathan Bossart wrote: >>> On Tue, Mar 17, 2026 at 02:04:11PM -0400, Greg Burd wrote: >>>> - * INDEX_ATTR_BITMAP_SUMMARIZED Columns included in summarizing >>>> indexes >>>> + * INDEX_ATTR_BITMAP_INDEXED Columns referenced by >>>> indexes >>>> + * INDEX_ATTR_BITMAP_SUMMARIZED Columns only included in >>>> summarizing indexes >>> >>>> - Bitmapset *summarizedattrs; /* columns with summarizing indexes */ >>>> + Bitmapset *indexedattrs; /* columns referenced by indexes */ >>>> + Bitmapset *summarizedattrs; /* columns only in summarizing indexes >>>> */ >>> >>> As before, the comment changes for the summarized-attr-related stuff seem >>> unnecessary. >> >> I disagree, the "only" is required to highlight the logic change here. >> Before this patch summarized attrs could overlap with indexed attrs, now >> it should not. This makes the logic a bit easier later in >> HeapUpdateHotAllowable(). > > My bad, you are right.
Hey Nathan, No worries, that's why we talk about it. :) >> So, we go from 3 calls to RelationGetIndexAttrBitmap() to 1, or at most 2 >> when there's a summarizing index (which is frequently the case). I meant to say that the common case (no summarizing indexes) we don't need more than the 1 bitmap, only in the less common case do we even need to get the *only* summarized attrs from relcache. >> This feels more logical, cleaner, and has less overhead but supports the >> same HOT logic. > > Nice. I'm happy that you agree! > -- > nathan best. -greg
