Re: [HACKERS] Patch: Write Amplification Reduction Method (WARM) - Mailing list pgsql-hackers

> @@ -234,6 +236,21 @@ index_beginscan(Relation heapRelation,
>       scan->heapRelation = heapRelation;
>       scan->xs_snapshot = snapshot;
>
> +     /*
> +      * If the index supports recheck, make sure that index tuple is saved
> +      * during index scans.
> +      *
> +      * XXX Ideally, we should look at all indexes on the table and check if
> +      * WARM is at all supported on the base table. If WARM is not supported
> +      * then we don't need to do any recheck. RelationGetIndexAttrBitmap() does
> +      * do that and sets rd_supportswarm after looking at all indexes. But we
> +      * don't know if the function was called earlier in the session when we're
> +      * here. We can't call it now because there exists a risk of causing
> +      * deadlock.
> +      */
> +     if (indexRelation->rd_amroutine->amrecheck)
> +             scan->xs_want_itup = true;
> +
>       return scan;
>  }

I didn't like this comment very much.  But it's not necessary: you have
already given relcache responsibility for setting rd_supportswarm.  The
only problem seems to be that you set it in RelationGetIndexAttrBitmap
instead of RelationGetIndexList, but it's not clear to me why.

I noticed that nbtinsert.c and nbtree.c have a bunch of new includes
that they don't actually need.  Let's remove those.  nbtutils.c does
need them because of btrecheck(). 

Speaking of which:

I have already commented about the executor involvement in btrecheck();
that doesn't seem good.  I previously suggested to pass the EState down
from caller, but that's not a great idea either since you still need to
do the actual FormIndexDatum.  I now think that a workable option would
be to compute the values/isnulls arrays so that btrecheck gets them
already computed.