Re: [PATCH] Don't block HOT update by BRIN index

po 12. 7. 2021 v 23:15 odesílatel Tomas Vondra
<tomas(dot)vondra(at)enterprisedb(dot)com> napsal:
>
>
>
> On 7/12/21 11:02 PM, Alvaro Herrera wrote:
> > On 2021-Jul-12, Josef Šimánek wrote:
> >
> >>> 2) Do we actually need to calculate and store hotblockingattrs
> >>> separately in RelationGetIndexAttrBitmap? It seems to me it's either
> >>> NULL (with amhotblocking=false) or equal to indexattrs. So why not to
> >>> just get rid of hotblockingattr and rd_hotblockingattr, and do something
> >>> like
> >>>
> >>> case INDEX_ATTR_BITMAP_HOT_BLOCKING:
> >>> return (amhotblocking) ? bms_copy(rel->rd_hotblockingattr) : NULL;
> >>>
> >>> I haven't tried, so maybe I'm missing something?
> >>
> >> relation->rd_indexattr is currently not used (at least I have not
> >> found anything) for anything, except looking if other values are
> >> already loaded.
> >
> > Oh, that's interesting. What this means is that INDEX_ATTR_BITMAP_ALL
> > is no longer used; its uses must have all been replaced by something
> > else. It seems the only one that currently exists is for HOT in
> > heap_update, which this patch replaces with the new one. In a quick
> > search, no external code depends on it, so I'd be inclined to just
> > remove it ...
> >
> > I think a boolean is much simpler. Consider a table with 1600 columns :-)
> >
>
> I'm not sure how to verify no external code depends on that flag. I have
> no idea if there's a plausible use case for it, though.

I tried GitHub search before to ensure at least it is not a widely
used "API". There were no results outside of PostgreSQL code itself in
first 10 pages of results.

> Even with 1600 columns the amount of wasted memory is only about 200B,
> which is not that bad I think. Not great, not terrible.
>
> OTOH most tables won't have any BRIN indexes, in which case indexattr
> and hotblockingattr are guaranteed to be exactly the same. So maybe
> that's something we could leverage - we need to calculate the "hot"
> bitmap, and in most cases we can use it for indexattr too.
>
> Maybe let's leave that for a separate patch, though?
>
>
> regards
>
> --
> Tomas Vondra
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company