From a210be772047575331fb6b0ab7b72043f81452ba Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 21 Feb 2011 21:18:04 -0500 Subject: Fix dangling-pointer problem in before-row update trigger processing. ExecUpdate checked for whether ExecBRUpdateTriggers had returned a new tuple value by seeing if the returned tuple was pointer-equal to the old one. But the "old one" was in estate->es_junkFilter's result slot, which would be scribbled on if we had done an EvalPlanQual update in response to a concurrent update of the target tuple; therefore we were comparing a dangling pointer to a live one. Given the right set of circumstances we could get a false match, resulting in not forcing the tuple to be stored in the slot we thought it was stored in. In the case reported by Maxim Boguk in bug #5798, this led to "cannot extract system attribute from virtual tuple" failures when trying to do "RETURNING ctid". I believe there is a very-low-probability chance of more serious errors, such as generating incorrect index entries based on the original rather than the trigger-modified version of the row. In HEAD, change all of ExecBRInsertTriggers, ExecIRInsertTriggers, ExecBRUpdateTriggers, and ExecIRUpdateTriggers so that they continue to have similar APIs. In the back branches I just changed ExecBRUpdateTriggers, since there is no bug in the ExecBRInsertTriggers case. --- src/backend/executor/nodeModifyTable.c | 103 ++++++--------------------------- 1 file changed, 18 insertions(+), 85 deletions(-) (limited to 'src/backend/executor/nodeModifyTable.c') diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 42662bdc461..12a5b2a8953 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -199,60 +199,26 @@ ExecInsert(TupleTableSlot *slot, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_before_row) { - HeapTuple newtuple; + slot = ExecBRInsertTriggers(estate, resultRelInfo, slot); - newtuple = ExecBRInsertTriggers(estate, resultRelInfo, tuple); - - if (newtuple == NULL) /* "do nothing" */ + if (slot == NULL) /* "do nothing" */ return NULL; - if (newtuple != tuple) /* modified by Trigger(s) */ - { - /* - * Put the modified tuple into a slot for convenience of routines - * below. We assume the tuple was allocated in per-tuple memory - * context, and therefore will go away by itself. The tuple table - * slot should not try to clear it. - */ - TupleTableSlot *newslot = estate->es_trig_tuple_slot; - TupleDesc tupdesc = RelationGetDescr(resultRelationDesc); - - if (newslot->tts_tupleDescriptor != tupdesc) - ExecSetSlotDescriptor(newslot, tupdesc); - ExecStoreTuple(newtuple, newslot, InvalidBuffer, false); - slot = newslot; - tuple = newtuple; - } + /* trigger might have changed tuple */ + tuple = ExecMaterializeSlot(slot); } /* INSTEAD OF ROW INSERT Triggers */ if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_instead_row) { - HeapTuple newtuple; + slot = ExecIRInsertTriggers(estate, resultRelInfo, slot); - newtuple = ExecIRInsertTriggers(estate, resultRelInfo, tuple); - - if (newtuple == NULL) /* "do nothing" */ + if (slot == NULL) /* "do nothing" */ return NULL; - if (newtuple != tuple) /* modified by Trigger(s) */ - { - /* - * Put the modified tuple into a slot for convenience of routines - * below. We assume the tuple was allocated in per-tuple memory - * context, and therefore will go away by itself. The tuple table - * slot should not try to clear it. - */ - TupleTableSlot *newslot = estate->es_trig_tuple_slot; - TupleDesc tupdesc = RelationGetDescr(resultRelationDesc); - - if (newslot->tts_tupleDescriptor != tupdesc) - ExecSetSlotDescriptor(newslot, tupdesc); - ExecStoreTuple(newtuple, newslot, InvalidBuffer, false); - slot = newslot; - tuple = newtuple; - } + /* trigger might have changed tuple */ + tuple = ExecMaterializeSlot(slot); newId = InvalidOid; } @@ -533,31 +499,14 @@ ExecUpdate(ItemPointer tupleid, if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_update_before_row) { - HeapTuple newtuple; + slot = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, + tupleid, slot); - newtuple = ExecBRUpdateTriggers(estate, epqstate, resultRelInfo, - tupleid, tuple); - - if (newtuple == NULL) /* "do nothing" */ + if (slot == NULL) /* "do nothing" */ return NULL; - if (newtuple != tuple) /* modified by Trigger(s) */ - { - /* - * Put the modified tuple into a slot for convenience of routines - * below. We assume the tuple was allocated in per-tuple memory - * context, and therefore will go away by itself. The tuple table - * slot should not try to clear it. - */ - TupleTableSlot *newslot = estate->es_trig_tuple_slot; - TupleDesc tupdesc = RelationGetDescr(resultRelationDesc); - - if (newslot->tts_tupleDescriptor != tupdesc) - ExecSetSlotDescriptor(newslot, tupdesc); - ExecStoreTuple(newtuple, newslot, InvalidBuffer, false); - slot = newslot; - tuple = newtuple; - } + /* trigger might have changed tuple */ + tuple = ExecMaterializeSlot(slot); } /* INSTEAD OF ROW UPDATE Triggers */ @@ -565,7 +514,6 @@ ExecUpdate(ItemPointer tupleid, resultRelInfo->ri_TrigDesc->trig_update_instead_row) { HeapTupleData oldtup; - HeapTuple newtuple; Assert(oldtuple != NULL); oldtup.t_data = oldtuple; @@ -573,29 +521,14 @@ ExecUpdate(ItemPointer tupleid, ItemPointerSetInvalid(&(oldtup.t_self)); oldtup.t_tableOid = InvalidOid; - newtuple = ExecIRUpdateTriggers(estate, resultRelInfo, - &oldtup, tuple); + slot = ExecIRUpdateTriggers(estate, resultRelInfo, + &oldtup, slot); - if (newtuple == NULL) /* "do nothing" */ + if (slot == NULL) /* "do nothing" */ return NULL; - if (newtuple != tuple) /* modified by Trigger(s) */ - { - /* - * Put the modified tuple into a slot for convenience of routines - * below. We assume the tuple was allocated in per-tuple memory - * context, and therefore will go away by itself. The tuple table - * slot should not try to clear it. - */ - TupleTableSlot *newslot = estate->es_trig_tuple_slot; - TupleDesc tupdesc = RelationGetDescr(resultRelationDesc); - - if (newslot->tts_tupleDescriptor != tupdesc) - ExecSetSlotDescriptor(newslot, tupdesc); - ExecStoreTuple(newtuple, newslot, InvalidBuffer, false); - slot = newslot; - tuple = newtuple; - } + /* trigger might have changed tuple */ + tuple = ExecMaterializeSlot(slot); } else { -- cgit v1.2.3