database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-05-15 19:15:29 +03:00
Code

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

Browse Source 
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 ff11e7f4b9ae 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 ff11e7f4b9ae was merged
This commit is contained in:
Andres Freund 2019-10-04 11:59:34 -07:00
parent c025165da9
commit 60e97d63e5
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)); Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
if (fdw_trigtuple == NULL) if (fdw_trigtuple == NULL)
{ {
TupleTableSlot *newSlot; TupleTableSlot *epqslot_candidate = NULL;
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid, if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
LockTupleExclusive, slot, &newSlot)) LockTupleExclusive, slot, &epqslot_candidate))
return false; return false;
/* /*
@ -2784,9 +2784,9 @@ ExecBRDeleteTriggers(EState *estate, EPQState *epqstate,
* function requested for the updated tuple, skip the trigger * function requested for the updated tuple, skip the trigger
* execution. * execution.
*/ */
if (newSlot != NULL && epqslot != NULL) if (epqslot_candidate != NULL && epqslot != NULL)
{ {
*epqslot = newSlot; *epqslot = epqslot_candidate;
return false; return false;
} }
@ -3026,11 +3026,11 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid)); Assert(HeapTupleIsValid(fdw_trigtuple) ^ ItemPointerIsValid(tupleid));
if (fdw_trigtuple == NULL) if (fdw_trigtuple == NULL)
{ {
TupleTableSlot *newSlot = NULL; TupleTableSlot *epqslot_candidate = NULL;
/* get a copy of the on-disk tuple we are planning to update */ /* get a copy of the on-disk tuple we are planning to update */
if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid, if (!GetTupleForTrigger(estate, epqstate, relinfo, tupleid,
lockmode, oldslot, &newSlot)) lockmode, oldslot, &epqslot_candidate))
return false; /* cancel the update action */ 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 * nor our caller have any more interest in the prior contents of that
* slot. * 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); trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig);
@ -3311,17 +3314,17 @@ GetTupleForTrigger(EState *estate,
ItemPointer tid, ItemPointer tid,
LockTupleMode lockmode, LockTupleMode lockmode,
TupleTableSlot *oldslot, TupleTableSlot *oldslot,
TupleTableSlot **newSlot) TupleTableSlot **epqslot)
{ {
Relation relation = relinfo->ri_RelationDesc; Relation relation = relinfo->ri_RelationDesc;
if (newSlot != NULL) if (epqslot != NULL)
{ {
TM_Result test; TM_Result test;
TM_FailureData tmfd; TM_FailureData tmfd;
int lockflags = 0; int lockflags = 0;
*newSlot = NULL; *epqslot = NULL;
/* caller must pass an epqstate if EvalPlanQual is possible */ /* caller must pass an epqstate if EvalPlanQual is possible */
Assert(epqstate != NULL); Assert(epqstate != NULL);
@ -3361,21 +3364,20 @@ GetTupleForTrigger(EState *estate,
case TM_Ok: case TM_Ok:
if (tmfd.traversed) if (tmfd.traversed)
{ {
TupleTableSlot *epqslot; *epqslot = EvalPlanQual(epqstate,
relation,
epqslot = EvalPlanQual(epqstate, relinfo->ri_RangeTableIndex,
relation, oldslot);
relinfo->ri_RangeTableIndex,
oldslot);
/* /*
* If PlanQual failed for updated tuple - we must not * If PlanQual failed for updated tuple - we must not
* process this tuple! * process this tuple!
*/ */
if (TupIsNull(epqslot)) if (TupIsNull(*epqslot))
{
*epqslot = NULL;
return false; return false;
}
*newSlot = epqslot;
} }
break; break;

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

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