On Tue, Jul 19, 2022 at 6:35 PM Andrey Lepikhov
<a.lepikhov@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