From: Amit Langote <amitlangot...@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
v16-0001-Fast-COPY-FROM-into-the-foreign-or-sharded-table.patch
Description: v16-0001-Fast-COPY-FROM-into-the-foreign-or-sharded-table.patch
