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 20190624101616.GI1637@paquier.xyz
Whole thread Raw
In response to Re: Custom table AMs need to include heapam.h because of BulkInsertState  (David Rowley <david.rowley@2ndquadrant.com>)
Responses Re: Custom table AMs need to include heapam.h because of BulkInsertState
List pgsql-hackers
On Sat, Jun 15, 2019 at 12:25:12PM +1200, David Rowley wrote:
> With the attached version I'm just calling table_finish_bulk_insert()
> once per partition at the end of CopyFrom().  We've got an array with
> all the ResultRelInfos we touched in the proute, so it's mostly a
> matter of looping over that array and calling the function on each
> ResultRelInfo's ri_RelationDesc.  However, to make it more complex,
> PartitionTupleRouting is private to execPartition.c so we can't do
> this directly... After staring at my screen for a while, I decided to
> write a function that calls a callback function on each ResultRelInfo
> in the PartitionTupleRouting.

Don't take me bad, but I find the solution of defining and using a new
callback to call the table AM callback not really elegant, and keeping
all table AM callbacks called at a higher level than the executor
makes the code easier to follow.  Shouldn't we try to keep any calls
to table_finish_bulk_insert() within copy.c for each partition
instead?

> The other thing I noticed is that we call
> table_finish_bulk_insert(cstate->rel, ti_options); in copy.c
> regardless of if we've done any bulk inserts or not. Perhaps that
> should be under an if (insertMethod != CIM_SINGLE)

Yeah, good point.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Darafei "Komяpa" Praliaskouski
Date:
Subject: GiST "choose subtree" support function to inline penalty
Next
From: David Rowley
Date:
Subject: Re: Custom table AMs need to include heapam.h because of BulkInsertState