RE: [POC] Fast COPY FROM command for the table with foreign partitions

From: Amit Langote <amitlangote09@gmail.com>
> Think I have mentioned upthread that this looks better as:
> 
> if (rootResultRelInfo->ri_usesMultiInsert)
>     leaf_part_rri->ri_usesMultiInsert = ExecMultiInsertAllowed(leaf_part_rri);
> 
> This keeps the logic confined to ExecInitPartitionInfo() where it
> belongs.  No point in burdening other callers of
> ExecMultiInsertAllowed() in deciding whether or not it should pass a
> valid value for the 2nd parameter.

Oh, that's a good idea.  (Why didn't I think of such a simple idea?)



> Maybe a bit late realizing this, but why does BeginForeignCopy()
> accept a ModifyTableState pointer whereas maybe just an EState pointer
> will do?  I can't imagine why an FDW would want to look at the
> ModifyTableState.  Case in point, I see that
> postgresBeginForeignCopy() only uses the EState from the
> ModifyTableState passed to it.  I think the ResultRelInfo that's being
> passed to the Copy APIs contains most of the necessary information.

You're right.  COPY is not under the control of a ModifyTable plan, so it's strange to pass ModifyTableState.


> Also, EndForeignCopy() seems fine with just receiving the EState.

I think this can have the ResultRelInfo like EndForeignInsert() and EndForeignModify() to correspond to the Begin function: "begin/end COPYing into this relation."


>     /*
>      * Check whether we support copying data out of the specified relation,
>      * unless the caller also passed a non-NULL data_dest_cb, in which case,
>      * the callback will take care of it
>      */
>     if (rel != NULL && rel->rd_rel->relkind != RELKIND_RELATION &&
>         data_dest_cb == NULL)
> 
> I just checked that this works or at least doesn't break any newly added tests.

Good idea, too.  The code has become more readable.

Thank you a lot.  Your other comments that are not mentioned above are also reflected.  The attached patch passes the postgres_fdw regression test.


Regards
Takayuki Tsunakawa