Re: heapam_index_build_range_scan's anyvisible

On Mon, Jun 10, 2019 at 2:56 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

> > Currently, all AM needs to build HeapTuple in
> > index_build_range_scan function. I looked into all the callback functions
> > and only htup->t_self is used from heaptuple in all the functions
> (unless I
> > missed something). So, if seems fine will be happy to write patch to make
> > that argument ItemPointer instead of HeapTuple?
>
> I wonder if it should better be the slot? It's not inconceivable that
> some AMs could benefit from that. Although it'd add some complication
> to the heap HeapTupleIsHeapOnly case.
>

I like the slot suggestion, only if can push FormIndexDatum() out of AM
code as a result and pass slot to the callback. Not sure what else can it
help. HeapTupleIsHeapOnly possibly can be made to work with slot similar to
current hack of updating the t_self and slot's tid field, maybe.

Index callback using the slot can form the index datum. Though that would
mean every Index AM callback function needs to do it, not sure which way is
better. Plus, need to create ExecutorState for the same. With current setup
every AM needs to do. Feels good if belongs to indexing code though instead
of AM.

Currently, index build needing to create ExecutorState and all at AM layer
seems not nice either. Maybe there is quite generic logic here and possible
can be extracted into common code which either most of AM leverage. Or
possibly the API itself can be simplified to get minimum input from AM and
have rest of flow/machinery at upper layer. As Robert pointed at start of
thread at heart its pretty simple flow and possibly will remain same for
any AM.