Re: Fast COPY FROM based on batch insert

On 3/22/22 06:54, Etsuro Fujita wrote:
> On Fri, Jun 4, 2021 at 5:26 PM Andrey Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
>> We still have slow 'COPY FROM' operation for foreign tables in current
>> master.
>> Now we have a foreign batch insert operation And I tried to rewrite the
>> patch [1] with this machinery.
> 
> The patch has been rewritten to something essentially different, but
> no one reviewed it.  (Tsunakawa-san gave some comments without looking
> at it, though.)  So the right status of the patch is “Needs review”,
> rather than “Ready for Committer”?  Anyway, here are a few review
> comments from me:
> 
> * I don’t think this assumption is correct:
> 
> @@ -359,6 +386,12 @@ CopyMultiInsertBufferFlush(CopyMultiInsertInfo *miinfo,
>                   (resultRelInfo->ri_TrigDesc->trig_insert_after_row ||
>                    resultRelInfo->ri_TrigDesc->trig_insert_new_table))
>          {
> +           /*
> +            * AFTER ROW triggers aren't allowed with the foreign bulk insert
> +            * method.
> +            */
> +           Assert(resultRelInfo->ri_RelationDesc->rd_rel->relkind !=
> RELKIND_FOREIGN_TABLE);
> +
> 
> In postgres_fdw we disable foreign batch insert when the target table
> has AFTER ROW triggers, but the core allows it even in that case.  No?
Agree

> * To allow foreign multi insert, the patch made an invasive change to
> the existing logic to determine whether to use multi insert for the
> target relation, adding a new member ri_usesMultiInsert to the
> ResultRelInfo struct, as well as introducing a new function
> ExecMultiInsertAllowed().  But I’m not sure we really need such a
> change.  Isn’t it reasonable to *adjust* the existing logic to allow
> foreign multi insert when possible?
Of course, such approach would look much better, if we implemented it. 
I'll ponder how to do it.

> I didn’t finish my review, but I’ll mark this as “Waiting on Author”.
I rebased the patch onto current master. Now it works correctly. I'll 
mark it as "Waiting for review".

-- 
regards,
Andrey Lepikhov
Postgres Professional