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/commands/copy.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'src/backend/commands/copy.c') diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index cac11a6c641..44f568f3964 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -1836,7 +1836,7 @@ CopyFrom(CopyState cstate) ResultRelInfo *resultRelInfo; EState *estate = CreateExecutorState(); /* for ExecConstraints() */ ExprContext *econtext; - TupleTableSlot *slot; + TupleTableSlot *myslot; MemoryContext oldcontext = CurrentMemoryContext; ErrorContextCallback errcontext; CommandId mycid = GetCurrentCommandId(true); @@ -1932,8 +1932,10 @@ CopyFrom(CopyState cstate) estate->es_result_relation_info = resultRelInfo; /* Set up a tuple slot too */ - slot = ExecInitExtraTupleSlot(estate); - ExecSetSlotDescriptor(slot, tupDesc); + myslot = ExecInitExtraTupleSlot(estate); + ExecSetSlotDescriptor(myslot, tupDesc); + /* Triggers might need a slot as well */ + estate->es_trig_tuple_slot = ExecInitExtraTupleSlot(estate); /* Prepare to catch AFTER triggers. */ AfterTriggerBeginQuery(); @@ -1960,6 +1962,7 @@ CopyFrom(CopyState cstate) for (;;) { + TupleTableSlot *slot; bool skip_tuple; Oid loaded_oid = InvalidOid; @@ -1983,32 +1986,28 @@ CopyFrom(CopyState cstate) /* Triggers and stuff need to be invoked in query context. */ MemoryContextSwitchTo(oldcontext); + /* Place tuple in tuple slot --- but slot shouldn't free it */ + slot = myslot; + ExecStoreTuple(tuple, slot, InvalidBuffer, false); + skip_tuple = false; /* BEFORE ROW INSERT Triggers */ if (resultRelInfo->ri_TrigDesc && resultRelInfo->ri_TrigDesc->trig_insert_before_row) { - HeapTuple newtuple; - - newtuple = ExecBRInsertTriggers(estate, resultRelInfo, tuple); + slot = ExecBRInsertTriggers(estate, resultRelInfo, slot); - if (newtuple == NULL) /* "do nothing" */ + if (slot == NULL) /* "do nothing" */ skip_tuple = true; - else if (newtuple != tuple) /* modified by Trigger(s) */ - { - heap_freetuple(tuple); - tuple = newtuple; - } + else /* trigger might have changed tuple */ + tuple = ExecMaterializeSlot(slot); } if (!skip_tuple) { List *recheckIndexes = NIL; - /* Place tuple in tuple slot */ - ExecStoreTuple(tuple, slot, InvalidBuffer, false); - /* Check the constraints of the tuple */ if (cstate->rel->rd_att->constr) ExecConstraints(resultRelInfo, slot, estate); -- cgit v1.2.3