On Tue, Jul 19, 2022 at 6:35 PM Andrey Lepikhov <a.lepik...@postgrespro.ru> wrote: > On 18/7/2022 13:22, Etsuro Fujita wrote: > > I rewrote the decision logic to something much simpler and much less > > invasive, which reduces the patch size significantly. Attached is an > > updated patch. What do you think about that?
> maybe you forgot this code: > /* > * If a partition's root parent isn't allowed to use it, neither is the > * partition. > */ > if (rootResultRelInfo->ri_usesMultiInsert) > leaf_part_rri->ri_usesMultiInsert = > ExecMultiInsertAllowed(leaf_part_rri); I think the patch accounts for that. Consider this bit to determine whether to use batching for the partition chosen by ExecFindPartition(): @@ -910,12 +962,14 @@ CopyFrom(CopyFromState cstate) /* * Disable multi-inserts when the partition has BEFORE/INSTEAD - * OF triggers, or if the partition is a foreign partition. + * OF triggers, or if the partition is a foreign partition + * that can't use batching. */ leafpart_use_multi_insert = insertMethod == CIM_MULTI_CONDITION\ AL && !has_before_insert_row_trig && !has_instead_insert_row_trig && - resultRelInfo->ri_FdwRoutine == NULL; + (resultRelInfo->ri_FdwRoutine == NULL || + resultRelInfo->ri_BatchSize > 1); If the root parent isn't allowed to use batching, then we have insertMethod=CIM_SINGLE for the parent before we get here. So in that case we have leafpart_use_multi_insert=false for the chosen partition, meaning that the partition isn't allowed to use batching, either. (The patch just extends the existing decision logic to the foreign-partition case.) > Also, maybe to describe in documentation, if the value of batch_size is > more than 1, the ExecForeignBatchInsert routine have a chance to be called? Yeah, but I think that is the existing behavior, and that the patch doesn't change the behavior, so I would leave that for another patch. Thanks for reviewing! Best regards, Etsuro Fujita