Re: Ignoring BRIN for HOT updates (was: -udpates seems broken)

On Wed, 22 Feb 2023 at 14:14, Matthias van de Meent
<boekewurm+postgres(at)gmail(dot)com> wrote:
>
> On Wed, 22 Feb 2023 at 13:15, Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> >
> > On 2/20/23 19:15, Matthias van de Meent wrote:
> > > Thanks. Based on feedback, attached is v2 of the patch, with as
> > > significant changes:
> > >
> > > - We don't store the columns we mention in predicates of summarized
> > > indexes in the hotblocking column anymore, they are stored in the
> > > summarized columns bitmap instead. This further reduces the chance of
> > > failiing to apply HOT with summarizing indexes.
> >
> > Interesting idea. I need to think about the correctness, but AFAICS it
> > should work. Do we have any tests covering such cases?
>
> There is a test that checks that an update to the predicated column
> does update the index (on table brin_hot_2). However, the description
> was out of date, so I've updated that in v4.
>
> > > - The heaptuple header bit for summarized update in inserted tuples is
> > > replaced with passing an out parameter. This simplifies the logic and
> > > decreases chances of accidentally storing incorrect data.
> > >
> >
> > OK.
> >
> > 0002 proposes a minor RelationGetIndexPredicate() tweak, getting rid of
> > the repeated if/else branches. Feel free to discard, if you think the v2
> > approach is better.
>
> I agree that this is better, it's included in v4 of the patch, as attached.

I think that the v4 patch solves all comments up to now; and
considering that most of this patch was committed but then reverted
due to an issue in v15, and that said issue is fixed in this patch,
I'm marking this as ready for committer.

Tomas, would you be up for that?

Kind regards,

Matthias van de Meent