I've run into what seems to be a bug in ExecInsert() that causes a crash
when inserting multiple rows into a partitioned table that each go into
different partitions with different tuple descriptors. Crash occurs if
ExecInsert() returns without resetting estate->es_result_relation_info
back to the root table's resultRelInfo. For example, if a BR trigger on a
partition returns NULL for a row.
Crashing example:
create table p (a int, b text) partition by list (a);
create table p12 (b text, a int);
-- p12 has different tuple descriptor than p
alter table p attach partition p12 for values in (1, 2);
create table p4 partition of p for values in (4);
create function blackhole () returns trigger as $$ begin return NULL; end;
$$ language plpgsql;
create trigger blackhole before insert on p12 for each row execute
procedure blackhole();
insert into p values (1, 'b'), (4, 'a');
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
Crash is caused because we enter into ExecFindPartition with p12's
resultRelInfo as if it correponds to the root table. That happens because
we didn't reset estate->es_result_relation_info, which had been set to
p12's resultRelInfo to point back to the original resultRelInfo (that is,
p's) before returning like below:
slot = ExecIRInsertTriggers(estate, resultRelInfo, slot);
if (slot == NULL) /* "do nothing" */
return NULL;
There are other places where we prematurely return like that.
Attached a patch to fix that, which would need to be back-patched to 10.
Thanks,
Amit
diff --git a/src/backend/executor/nodeModifyTable.c
b/src/backend/executor/nodeModifyTable.c
index c32928d9bd..0d6326d1de 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -281,7 +281,7 @@ ExecInsert(ModifyTableState *mtstate,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
+ saved_resultRelInfo = resultRelInfo = estate->es_result_relation_info;
/* Determine the partition to heap_insert the tuple into */
if (mtstate->mt_partition_tuple_routing)
@@ -408,7 +408,10 @@ ExecInsert(ModifyTableState *mtstate,
slot = ExecBRInsertTriggers(estate, resultRelInfo, slot);
if (slot == NULL) /* "do nothing" */
- return NULL;
+ {
+ result = NULL;
+ goto restore_estate_result_rel;
+ }
/* trigger might have changed tuple */
tuple = ExecMaterializeSlot(slot);
@@ -421,7 +424,10 @@ ExecInsert(ModifyTableState *mtstate,
slot = ExecIRInsertTriggers(estate, resultRelInfo, slot);
if (slot == NULL) /* "do nothing" */
- return NULL;
+ {
+ result = NULL;
+ goto restore_estate_result_rel;
+ }
/* trigger might have changed tuple */
tuple = ExecMaterializeSlot(slot);
@@ -439,7 +445,10 @@ ExecInsert(ModifyTableState *mtstate,
planSlot);
if (slot == NULL) /* "do nothing" */
- return NULL;
+ {
+ result = NULL;
+ goto restore_estate_result_rel;
+ }
/* FDW might have changed tuple */
tuple = ExecMaterializeSlot(slot);
@@ -495,7 +504,7 @@ ExecInsert(ModifyTableState *mtstate,
* No need though if the tuple has been routed, and a BR trigger
* doesn't exist.
*/
- if (saved_resultRelInfo != NULL &&
+ if (saved_resultRelInfo != resultRelInfo &&
!(resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_insert_before_row))
check_partition_constr = false;
@@ -544,7 +553,8 @@ ExecInsert(ModifyTableState *mtstate,
estate, canSetTag, &returning))
{
InstrCountFiltered2(&mtstate->ps, 1);
- return returning;
+ result = returning;
+ goto restore_estate_result_rel;
}
else
goto vlock;
@@ -559,7 +569,8 @@ ExecInsert(ModifyTableState *mtstate,
Assert(onconflict ==
ONCONFLICT_NOTHING);
ExecCheckTIDVisible(estate,
resultRelInfo, &conflictTid);
InstrCountFiltered2(&mtstate->ps, 1);
- return NULL;
+ result = NULL;
+ goto restore_estate_result_rel;
}
}
@@ -686,6 +697,7 @@ ExecInsert(ModifyTableState *mtstate,
if (resultRelInfo->ri_projectReturning)
result = ExecProcessReturning(resultRelInfo, slot, planSlot);
+restore_estate_result_rel:
if (saved_resultRelInfo)
estate->es_result_relation_info = saved_resultRelInfo;
