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

On Thu, 18 Jul 2019 at 11:36, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2019-07-18 11:29:37 +1200, David Rowley wrote:
> > On Wed, 17 Jul 2019 at 06:46, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > > 1) Have ExecFindPartition() return via a bool* whether the partition is
> > > being accessed for the first time. In copy.c push the partition onto
> > > a list of to-be-bulk-finished tables.
> > > 2) Add a execPartition.c function that returns all the used tables from
> > > a PartitionTupleRouting*.
> >
> > #2 seems better than #1 as it does not add overhead to ExecFindPartition().
>
> I don't see how #1 would add meaningful overhead compared to the other
> costs of that function. Wouldn't it just be adding if (isnew) *isnew =
> false; to the "/* ResultRelInfo already built */" branch, and the
> reverse to the else?

Yes

> That got to be several orders of magnitude cheaper
> than e.g. FormPartitionKeyDatum() which is unconditionally executed?

Probably.

However, I spent quite a bit of time trying to make that function as
fast as possible in v12, and since #2 seems like a perfectly good
alternative, I'd rather go with that than to add pollution to
ExecFindPartition's signature. Also, #2 seems better since it keeps
CopyFrom() from having to maintain a list. I think we all agreed
somewhere that that code is more complex than we'd all like it to be.

--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services