Re: Custom table AMs need to include heapam.h because ofBulkInsertState - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Custom table AMs need to include heapam.h because ofBulkInsertState
Date
Msg-id 20190607022949.GC1736@paquier.xyz
Whole thread Raw
In response to Re: Custom table AMs need to include heapam.h because of BulkInsertState  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Custom table AMs need to include heapam.h because of BulkInsertState
List pgsql-hackers
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@paquier.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

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: tableam: abstracting relation sizing code
Next
From: Amit Langote
Date:
Subject: Re: hyrax vs. RelationBuildPartitionDesc