rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix crash caused by EPQ happening with a before update trigger present. 

When ExecBRUpdateTriggers()'s GetTupleForTrigger() follows an EPQ
chain the former needs to run the result tuple through the junkfilter
again, and update the slot containing the new version of the tuple to
contain that new version. The input tuple may already be in the
junkfilter's output slot, which used to be OK - we don't need the
previous version anymore. Unfortunately ff11e7f4b9 started to use
ExecCopySlot() to update newslot, and ExecCopySlot() doesn't support
copying a slot into itself, leading to a slot in a corrupt
state, which then can cause crashes or other symptoms.

Fix this by skipping the ExecCopySlot() when copying into itself.

While we could have easily made ExecCopySlot() handle that case, it
seems better to add an assert forbidding doing so instead. As the goal
of copying might be to make the contents of one slot independent from
another, it seems failure prone to handle doing so silently.

A follow-up commit will add tests for the obviously under-covered
combination of EPQ and triggers. Done as a separate commit as it might
make sense to backpatch them further than this bug.

Also remove confusion with confusing variable names for slots in
ExecBRDeleteTriggers() and ExecBRUpdateTriggers().

Bug: #16036
Reported-By: Антон Власов
Author: Andres Freund
Discussion: https://postgr.es/m/16036-28184c90d952fb7f@postgresql.org
Backpatch: 12-, where ff11e7f4b9 was merged
This commit is contained in:
Andres Freund 2019-10-04 11:59:34 -07:00
parent a586cc4b6c
commit d986d4e87f
2 changed files with 24 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
src/backend/commands/trigger.c
View File

@ -2773,10 +2773,10 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
if (fdw_trigtuple == NULL)
{
TupleTableSlot *newSlot;
TupleTableSlot *epqslot_candidate = NULL;
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
LockTupleExclusive, slot, &newSlot))
LockTupleExclusive, slot, &epqslot_candidate))
return false;
/*
@ -2784,9 +2784,9 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
* function requested for the updated tuple, skip the trigger
* execution.
*/
if (newSlot != NULL && epqslot != NULL)
if (epqslot_candidate != NULL && epqslot != NULL)
{
*epqslot = newSlot;
*epqslot = epqslot_candidate;
return false;
}
@ -3026,11 +3026,11 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
if (fdw_trigtuple == NULL)
{
TupleTableSlot *newSlot = NULL;
TupleTableSlot *epqslot_candidate = NULL;
/* get a copy of the on-disk tuple we are planning to update */
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
lockmode, oldslot, &newSlot))
lockmode, oldslot, &epqslot_candidate))
return false; /* cancel the update action */
/*
@ -3045,11 +3045,14 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
* nor our caller have any more interest in the prior contents of that
* slot.
*/
if (newSlot != NULL)
if (epqslot_candidate != NULL)
{
TupleTableSlot *slot = ExecFilterJunk(relinfo->ri_junkFilter, newSlot);
TupleTableSlot *epqslot_clean;
ExecCopySlot(newslot, slot);
epqslot_clean = ExecFilterJunk(relinfo->ri_junkFilter, epqslot_candidate);
if (newslot != epqslot_clean)
ExecCopySlot(newslot, epqslot_clean);
}
trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig);
@ -3311,17 +3314,17 @@ GetTupleForTrigger(EState *estate,
ItemPointer tid,
LockTupleMode lockmode,
TupleTableSlot *oldslot,
TupleTableSlot **newSlot)
TupleTableSlot **epqslot)
{
Relation relation = relinfo->ri_RelationDesc;
if (newSlot != NULL)
if (epqslot != NULL)
{
TM_Result test;
TM_FailureData tmfd;
int lockflags = 0;
*newSlot = NULL;
*epqslot = NULL;
/* caller must pass an epqstate if EvalPlanQual is possible */
Assert(epqstate != NULL);
@ -3361,21 +3364,20 @@ GetTupleForTrigger(EState *estate,
case TM_Ok:
if (tmfd.traversed)
{
TupleTableSlot *epqslot;
epqslot = EvalPlanQual(epqstate,
relation,
relinfo->ri_RangeTableIndex,
oldslot);
*epqslot = EvalPlanQual(epqstate,
relation,
relinfo->ri_RangeTableIndex,
oldslot);
/*
* If PlanQual failed for updated tuple - we must not
* process this tuple!
*/
if (TupIsNull(epqslot))
if (TupIsNull(*epqslot))
{
*epqslot = NULL;
return false;
*newSlot = epqslot;
}
}
break;

1
src/include/executor/tuptable.h
View File

@ -476,6 +476,7 @@ static inline TupleTableSlot *
ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot)
{
Assert(!TTS_EMPTY(srcslot));
AssertArg(srcslot != dstslot);
dstslot->tts_ops->copyslot(dstslot, srcslot);