Re: Custom table AMs need to include heapam.h because of BulkInsertState

On Tue, Jun 04, 2019 at 10:18:03AM -0400, Robert Haas wrote:
> On Sat, Jun 1, 2019 at 3:09 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> I am fine to live with the dependency with vacuum.h as it is not that
>> strange. However for BulkInsertState we get a hard dependency with a
>> heap-related area and it seems to me that we had better move that part
>> out of heapam.c, as we want a clear dependency cut with the heap AM
>> for any new custom table AM.
>
> Yeah, I noticed this, too. +1 for doing something about it. Not sure
> exactly what is the best approach.

One thing which is a bit tricky is that for example with COPY FROM we
have a code path which is able to release a buffer held by the bulk
insert state. So I think that we could get easily out by combining
the bistate free path with finish_bulk_insert, create the bistate
within the AM when doing a single or multi tuple insert, and having
one extra callback to release a buffer held. Still this last bit does
not completely feel right in terms of flexibility and readability.

Note as well that we never actually use bistate when calling
table_tuple_insert_speculative() on HEAD. I guess that the argument
is here for consistency with the tuple_insert callback. Could we do
something separately about that?
--
Michael