Hi,
Currently the tuple returned by INSTEAD OF triggers on DELETEs is only
used to determine whether to pretend that the DELETE happened or not,
which is often not helpful enough; for example, the actual tuple might
have been concurrently UPDATEd by another transaction and one or more of
the columns now hold values different from those in the planSlot tuple.
Attached is an example case which is impossible to properly implement
under the current behavior. For what it's worth, the current behavior
seems to be an accident; before INSTEAD OF triggers either the tuple was
already locked (in case of BEFORE triggers) or the actual pre-DELETE
version of the tuple was fetched from the heap.
So I'm suggesting to change this behavior and allow INSTEAD OF DELETE
triggers to modify the OLD tuple and use that for RETURNING instead of
returning the tuple in planSlot. Attached is a WIP patch implementing that.
Is there any reason why we wouldn't want to change the current behavior?
.m
BEGIN;
CREATE OR REPLACE FUNCTION foof() RETURNS TRIGGER AS $$
BEGIN
-- imagine someone concurrently incremented counter here
OLD.counter := OLD.counter + 1;
RETURN OLD;
END
$$ LANGUAGE plpgsql;
CREATE TABLE foo(counter int NOT NULL);
CREATE VIEW foov AS SELECT * FROM foo;
CREATE TRIGGER foov_instead
INSTEAD OF DELETE ON foov
FOR EACH ROW
EXECUTE PROCEDURE foof();
INSERT INTO foo VALUES (0);
DELETE FROM foov RETURNING counter;
ROLLBACK;
*** a/src/backend/commands/trigger.c
--- b/src/backend/commands/trigger.c
***************
*** 2295,2307 **** ExecARDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
}
}
! bool
ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
! HeapTuple trigtuple)
{
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
TriggerData LocTriggerData;
! HeapTuple rettuple;
int i;
LocTriggerData.type = T_TriggerData;
--- 2295,2307 ----
}
}
! TupleTableSlot *
ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
! HeapTuple trigtuple, TupleTableSlot
*slot)
{
TriggerDesc *trigdesc = relinfo->ri_TrigDesc;
TriggerData LocTriggerData;
! HeapTuple rettuple = trigtuple;
int i;
LocTriggerData.type = T_TriggerData;
***************
*** 2333,2343 **** ExecIRDeleteTriggers(EState *estate, ResultRelInfo *relinfo,
relinfo->ri_TrigInstrument,
GetPerTupleMemoryContext(estate));
if (rettuple == NULL)
! return false; /* Delete was suppressed */
! if (rettuple != trigtuple)
! heap_freetuple(rettuple);
}
! return true;
}
void
--- 2333,2359 ----
relinfo->ri_TrigInstrument,
GetPerTupleMemoryContext(estate));
if (rettuple == NULL)
! return NULL; /* Delete was suppressed */
}
!
! if (rettuple != trigtuple)
! {
! /*
! * Return the modified tuple using the es_trig_tuple_slot. 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(relinfo->ri_RelationDesc);
!
! if (newslot->tts_tupleDescriptor != tupdesc)
! ExecSetSlotDescriptor(newslot, tupdesc);
! ExecStoreTuple(rettuple, newslot, InvalidBuffer, false);
! slot = newslot;
! }
!
! return slot;
}
void
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 573,585 **** ExecDelete(ItemPointer tupleid,
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_delete_instead_row)
{
! bool dodelete;
! Assert(oldtuple != NULL);
! dodelete = ExecIRDeleteTriggers(estate, resultRelInfo,
oldtuple);
! if (!dodelete) /* "do nothing" */
return NULL;
}
else if (resultRelInfo->ri_FdwRoutine)
{
--- 573,595 ----
if (resultRelInfo->ri_TrigDesc &&
resultRelInfo->ri_TrigDesc->trig_delete_instead_row)
{
! /*
! * Store the heap tuple into the tuple table slot, making sure
we have a
! * writable copy. We can use the trigger tuple slot.
! */
! slot = estate->es_trig_tuple_slot;
! if (slot->tts_tupleDescriptor !=
RelationGetDescr(resultRelationDesc))
! ExecSetSlotDescriptor(slot,
RelationGetDescr(resultRelationDesc));
! ExecStoreTuple(oldtuple, slot, InvalidBuffer, false);
! oldtuple = ExecMaterializeSlot(slot);
! slot = ExecIRDeleteTriggers(estate, resultRelInfo, oldtuple,
slot);
! if (slot == NULL) /* "do nothing" */
return NULL;
+
+ /* trigger might have changed tuple */
+ oldtuple = ExecMaterializeSlot(slot);
}
else if (resultRelInfo->ri_FdwRoutine)
{
***************
*** 719,732 **** ldelete:;
/* Process RETURNING if present */
if (resultRelInfo->ri_projectReturning)
{
- /*
- * We have to put the target tuple into a slot, which means
first we
- * gotta fetch it. We can use the trigger tuple slot.
- */
TupleTableSlot *rslot;
HeapTupleData deltuple;
Buffer delbuffer;
if (resultRelInfo->ri_FdwRoutine)
{
/* FDW must have provided a slot containing the deleted
row */
--- 729,750 ----
/* Process RETURNING if present */
if (resultRelInfo->ri_projectReturning)
{
TupleTableSlot *rslot;
HeapTupleData deltuple;
Buffer delbuffer;
+ /*
+ * If we fired an INSTEAD OF trigger, we should use the tuple
returned
+ * from said trigger for the RETURNING projections.
+ */
+ if (resultRelInfo->ri_TrigDesc &&
+ resultRelInfo->ri_TrigDesc->trig_delete_instead_row)
+ return ExecProcessReturning(resultRelInfo, slot,
planSlot);
+
+ /*
+ * Otherwise we have to to fetch the target tuple into a slot.
We can
+ * use the trigger tuple slot here as well.
+ */
if (resultRelInfo->ri_FdwRoutine)
{
/* FDW must have provided a slot containing the deleted
row */
*** a/src/include/commands/trigger.h
--- b/src/include/commands/trigger.h
***************
*** 154,162 **** extern void ExecARDeleteTriggers(EState *estate,
ResultRelInfo *relinfo,
ItemPointer tupleid,
HeapTuple fdw_trigtuple);
! extern bool ExecIRDeleteTriggers(EState *estate,
ResultRelInfo *relinfo,
! HeapTuple trigtuple);
extern void ExecBSUpdateTriggers(EState *estate,
ResultRelInfo *relinfo);
extern void ExecASUpdateTriggers(EState *estate,
--- 154,163 ----
ResultRelInfo *relinfo,
ItemPointer tupleid,
HeapTuple fdw_trigtuple);
! extern TupleTableSlot *ExecIRDeleteTriggers(EState *estate,
ResultRelInfo *relinfo,
! HeapTuple trigtuple,
! TupleTableSlot *slot);
extern void ExecBSUpdateTriggers(EState *estate,
ResultRelInfo *relinfo);
extern void ExecASUpdateTriggers(EState *estate,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
