Skip to content

Commit

Permalink
Remove new structure member from ResultRelInfo.
Browse files Browse the repository at this point in the history
In commit ffbb7e6, I added a ModifyTableState member to ResultRelInfo
to save the owning ModifyTableState for use by nodeModifyTable.c when
performing batch inserts, but as pointed out by Tom Lane, that changed
the array stride of es_result_relations, and that would break any
previously-compiled extension code that accesses that array.  Fix by
removing that member from ResultRelInfo and instead adding a List member
at the end of EState to save such ModifyTableStates.

Per report from Tom Lane.  Back-patch to v14, like the previous commit;
I chose to apply the patch to HEAD as well, to make back-patching easy.

Discussion: http://postgr.es/m/4065383.1669395453%40sss.pgh.pa.us
  • Loading branch information
Etsuro Fujita committed Dec 8, 2022
1 parent dc3648f commit d43a97e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 29 deletions.
1 change: 0 additions & 1 deletion src/backend/executor/execMain.c
Expand Up @@ -1257,7 +1257,6 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ChildToRootMap = NULL;
resultRelInfo->ri_ChildToRootMapValid = false;
resultRelInfo->ri_CopyMultiInsertBuffer = NULL;
resultRelInfo->ri_ModifyTableState = NULL;
}

/*
Expand Down
7 changes: 0 additions & 7 deletions src/backend/executor/execPartition.c
Expand Up @@ -934,13 +934,6 @@ ExecInitRoutingInfo(ModifyTableState *mtstate,

Assert(partRelInfo->ri_BatchSize >= 1);

/*
* If doing batch insert, setup back-link so we can easily find the
* mtstate again.
*/
if (partRelInfo->ri_BatchSize > 1)
partRelInfo->ri_ModifyTableState = mtstate;

partRelInfo->ri_CopyMultiInsertBuffer = NULL;

/*
Expand Down
4 changes: 3 additions & 1 deletion src/backend/executor/execUtils.c
Expand Up @@ -127,9 +127,11 @@ CreateExecutorState(void)
estate->es_result_relations = NULL;
estate->es_opened_result_relations = NIL;
estate->es_tuple_routing_result_relations = NIL;
estate->es_insert_pending_result_relations = NIL;
estate->es_trig_target_relations = NIL;

estate->es_insert_pending_result_relations = NIL;
estate->es_insert_pending_modifytables = NIL;

estate->es_param_list_info = NULL;
estate->es_param_exec_vals = NULL;

Expand Down
31 changes: 16 additions & 15 deletions src/backend/executor/nodeModifyTable.c
Expand Up @@ -742,10 +742,12 @@ ExecInsert(ModifyTableState *mtstate,

/*
* If these are the first tuples stored in the buffers, add the
* target rel to the es_insert_pending_result_relations list,
* except in the case where flushing was done above, in which case
* the target rel would already have been added to the list, so no
* need to do this.
* target rel and the mtstate to the
* es_insert_pending_result_relations and
* es_insert_pending_modifytables lists respectively, execpt in
* the case where flushing was done above, in which case they
* would already have been added to the lists, so no need to do
* this.
*/
if (resultRelInfo->ri_NumSlots == 0 && !flushed)
{
Expand All @@ -754,6 +756,8 @@ ExecInsert(ModifyTableState *mtstate,
estate->es_insert_pending_result_relations =
lappend(estate->es_insert_pending_result_relations,
resultRelInfo);
estate->es_insert_pending_modifytables =
lappend(estate->es_insert_pending_modifytables, mtstate);
}
Assert(list_member_ptr(estate->es_insert_pending_result_relations,
resultRelInfo));
Expand Down Expand Up @@ -1445,12 +1449,14 @@ ldelete:;
static void
ExecPendingInserts(EState *estate)
{
ListCell *lc;
ListCell *l1,
*l2;

foreach(lc, estate->es_insert_pending_result_relations)
forboth(l1, estate->es_insert_pending_result_relations,
l2, estate->es_insert_pending_modifytables)
{
ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(lc);
ModifyTableState *mtstate = resultRelInfo->ri_ModifyTableState;
ResultRelInfo *resultRelInfo = (ResultRelInfo *) lfirst(l1);
ModifyTableState *mtstate = (ModifyTableState *) lfirst(l2);

Assert(mtstate);
ExecBatchInsert(mtstate, resultRelInfo,
Expand All @@ -1462,7 +1468,9 @@ ExecPendingInserts(EState *estate)
}

list_free(estate->es_insert_pending_result_relations);
list_free(estate->es_insert_pending_modifytables);
estate->es_insert_pending_result_relations = NIL;
estate->es_insert_pending_modifytables = NIL;
}

/*
Expand Down Expand Up @@ -3183,13 +3191,6 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
}
else
resultRelInfo->ri_BatchSize = 1;

/*
* If doing batch insert, setup back-link so we can easily find the
* mtstate again.
*/
if (resultRelInfo->ri_BatchSize > 1)
resultRelInfo->ri_ModifyTableState = mtstate;
}

/*
Expand Down
8 changes: 3 additions & 5 deletions src/include/nodes/execnodes.h
Expand Up @@ -524,9 +524,6 @@ typedef struct ResultRelInfo

/* for use by copyfrom.c when performing multi-inserts */
struct CopyMultiInsertBuffer *ri_CopyMultiInsertBuffer;

/* for use by nodeModifyTable.c when performing batch-inserts */
struct ModifyTableState *ri_ModifyTableState;
} ResultRelInfo;

/* ----------------
Expand Down Expand Up @@ -650,10 +647,11 @@ typedef struct EState
struct JitInstrumentation *es_jit_worker_instr;

/*
* The following list contains ResultRelInfos for foreign tables on which
* batch-inserts are to be executed.
* Lists of ResultRelInfos for foreign tables on which batch-inserts are
* to be executed and owning ModifyTableStates, stored in the same order.
*/
List *es_insert_pending_result_relations;
List *es_insert_pending_modifytables;
} EState;


Expand Down

0 comments on commit d43a97e

Please sign in to comment.