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

From: Amit Langote <amitlangote09(at)gmail(dot)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