Hi,
Here's a v2 fixing a silly bug with reusing the same variable in two
nested loops (worked for simple postgres_fdw cases, but "make check"
failed).
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
>From 494018fd3f2b983be474a85fc12fe3a4dbefa76b Mon Sep 17 00:00:00 2001
From: Tomas Vondra <to...@2ndquadrant.com>
Date: Fri, 4 Jun 2021 12:45:18 +0200
Subject: [PATCH 1/2] create copy of a descriptor for batching
---
src/backend/executor/nodeModifyTable.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 379b056310..c287a371a1 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -678,6 +678,8 @@ ExecInsert(ModifyTableState *mtstate,
*/
if (resultRelInfo->ri_BatchSize > 1)
{
+ TupleDesc tdesc;
+
/*
* If a certain number of tuples have already been accumulated, or
* a tuple has come for a different relation than that for the
@@ -703,13 +705,16 @@ ExecInsert(ModifyTableState *mtstate,
resultRelInfo->ri_BatchSize);
}
+ tdesc = CreateTupleDescCopy(slot->tts_tupleDescriptor);
+
resultRelInfo->ri_Slots[resultRelInfo->ri_NumSlots] =
- MakeSingleTupleTableSlot(slot->tts_tupleDescriptor,
+ MakeSingleTupleTableSlot(tdesc,
slot->tts_ops);
ExecCopySlot(resultRelInfo->ri_Slots[resultRelInfo->ri_NumSlots],
slot);
+
resultRelInfo->ri_PlanSlots[resultRelInfo->ri_NumSlots] =
- MakeSingleTupleTableSlot(planSlot->tts_tupleDescriptor,
+ MakeSingleTupleTableSlot(tdesc,
planSlot->tts_ops);
ExecCopySlot(resultRelInfo->ri_PlanSlots[resultRelInfo->ri_NumSlots],
planSlot);
--
2.31.1
>From 22a7a2c295ccce850da5f02d6239975057345b32 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <to...@2ndquadrant.com>
Date: Fri, 4 Jun 2021 12:59:47 +0200
Subject: [PATCH 2/2] initialize slots only once for batching
---
src/backend/executor/nodeModifyTable.c | 43 ++++++++++++++------------
src/include/nodes/execnodes.h | 1 +
2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index c287a371a1..81b3b522c2 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -678,8 +678,6 @@ ExecInsert(ModifyTableState *mtstate,
*/
if (resultRelInfo->ri_BatchSize > 1)
{
- TupleDesc tdesc;
-
/*
* If a certain number of tuples have already been accumulated, or
* a tuple has come for a different relation than that for the
@@ -705,19 +703,25 @@ ExecInsert(ModifyTableState *mtstate,
resultRelInfo->ri_BatchSize);
}
- tdesc = CreateTupleDescCopy(slot->tts_tupleDescriptor);
+ /* initialize the slot, if not already done */
+ if (resultRelInfo->ri_NumSlots >= resultRelInfo->ri_BatchInitialized)
+ {
+ TupleDesc tdesc = CreateTupleDescCopy(slot->tts_tupleDescriptor);
- resultRelInfo->ri_Slots[resultRelInfo->ri_NumSlots] =
- MakeSingleTupleTableSlot(tdesc,
- slot->tts_ops);
- ExecCopySlot(resultRelInfo->ri_Slots[resultRelInfo->ri_NumSlots],
- slot);
+ resultRelInfo->ri_Slots[resultRelInfo->ri_NumSlots] =
+ MakeSingleTupleTableSlot(tdesc,
+ slot->tts_ops);
+ ExecCopySlot(resultRelInfo->ri_Slots[resultRelInfo->ri_NumSlots],
+ slot);
- resultRelInfo->ri_PlanSlots[resultRelInfo->ri_NumSlots] =
- MakeSingleTupleTableSlot(tdesc,
- planSlot->tts_ops);
- ExecCopySlot(resultRelInfo->ri_PlanSlots[resultRelInfo->ri_NumSlots],
- planSlot);
+ resultRelInfo->ri_PlanSlots[resultRelInfo->ri_NumSlots] =
+ MakeSingleTupleTableSlot(tdesc,
+ planSlot->tts_ops);
+ ExecCopySlot(resultRelInfo->ri_PlanSlots[resultRelInfo->ri_NumSlots],
+ planSlot);
+
+ resultRelInfo->ri_BatchInitialized++;
+ }
resultRelInfo->ri_NumSlots++;
@@ -1039,12 +1043,6 @@ ExecBatchInsert(ModifyTableState *mtstate,
if (canSetTag && numInserted > 0)
estate->es_processed += numInserted;
-
- for (i = 0; i < numSlots; i++)
- {
- ExecDropSingleTupleTableSlot(slots[i]);
- ExecDropSingleTupleTableSlot(planSlots[i]);
- }
}
/* ----------------------------------------------------------------
@@ -3167,6 +3165,7 @@ ExecEndModifyTable(ModifyTableState *node)
*/
for (i = 0; i < node->mt_nrels; i++)
{
+ int j;
ResultRelInfo *resultRelInfo = node->resultRelInfo + i;
if (!resultRelInfo->ri_usesFdwDirectModify &&
@@ -3174,6 +3173,12 @@ ExecEndModifyTable(ModifyTableState *node)
resultRelInfo->ri_FdwRoutine->EndForeignModify != NULL)
resultRelInfo->ri_FdwRoutine->EndForeignModify(node->ps.state,
resultRelInfo);
+
+ for (j = 0; j < resultRelInfo->ri_NumSlots; j++)
+ {
+ ExecDropSingleTupleTableSlot(resultRelInfo->ri_Slots[i]);
+ ExecDropSingleTupleTableSlot(resultRelInfo->ri_PlanSlots[i]);
+ }
}
/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 7795a69490..1062a44ee1 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -463,6 +463,7 @@ typedef struct ResultRelInfo
/* batch insert stuff */
int ri_NumSlots; /* number of slots in the array */
int ri_BatchSize; /* max slots inserted in a single batch */
+ int ri_BatchInitialized; /* number of slots initialized */
TupleTableSlot **ri_Slots; /* input tuples for batch insert */
TupleTableSlot **ri_PlanSlots;
--
2.31.1
