Re: BUG #2166: attempted to update invisible tuple

I wrote:
> It looks to me like the correct test requires passing in the current
> command ID so we can check the tuple's cmax against it.

Er, cmin not cmax. Here's the patch against 8.1 if you need it.

regards, tom lane

Index: src/backend/commands/trigger.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/trigger.c,v
retrieving revision 1.195.2.1
diff -c -r1.195.2.1 trigger.c
*** src/backend/commands/trigger.c 22 Nov 2005 18:23:07 -0000 1.195.2.1
--- src/backend/commands/trigger.c 12 Jan 2006 21:10:09 -0000
***************
*** 1736,1742 ****
epqslot = EvalPlanQual(estate,
relinfo->ri_RangeTableIndex,
&update_ctid,
! update_xmax);
if (!TupIsNull(epqslot))
{
*tid = update_ctid;
--- 1736,1743 ----
epqslot = EvalPlanQual(estate,
relinfo->ri_RangeTableIndex,
&update_ctid,
! update_xmax,
! cid);
if (!TupIsNull(epqslot))
{
*tid = update_ctid;
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.256.2.4
diff -c -r1.256.2.4 execMain.c
*** src/backend/executor/execMain.c 22 Nov 2005 18:23:08 -0000 1.256.2.4
--- src/backend/executor/execMain.c 12 Jan 2006 21:10:09 -0000
***************
*** 1219,1225 ****
newSlot = EvalPlanQual(estate,
erm->rti,
&update_ctid,
! update_xmax);
if (!TupIsNull(newSlot))
{
slot = newSlot;
--- 1219,1226 ----
newSlot = EvalPlanQual(estate,
erm->rti,
&update_ctid,
! update_xmax,
! estate->es_snapshot->curcid);
if (!TupIsNull(newSlot))
{
slot = newSlot;
***************
*** 1527,1533 ****
epqslot = EvalPlanQual(estate,
resultRelInfo->ri_RangeTableIndex,
&update_ctid,
! update_xmax);
if (!TupIsNull(epqslot))
{
*tupleid = update_ctid;
--- 1528,1535 ----
epqslot = EvalPlanQual(estate,
resultRelInfo->ri_RangeTableIndex,
&update_ctid,
! update_xmax,
! estate->es_snapshot->curcid);
if (!TupIsNull(epqslot))
{
*tupleid = update_ctid;
***************
*** 1679,1685 ****
epqslot = EvalPlanQual(estate,
resultRelInfo->ri_RangeTableIndex,
&update_ctid,
! update_xmax);
if (!TupIsNull(epqslot))
{
*tupleid = update_ctid;
--- 1681,1688 ----
epqslot = EvalPlanQual(estate,
resultRelInfo->ri_RangeTableIndex,
&update_ctid,
! update_xmax,
! estate->es_snapshot->curcid);
if (!TupIsNull(epqslot))
{
*tupleid = update_ctid;
***************
*** 1826,1831 ****
--- 1829,1835 ----
* rti - rangetable index of table containing tuple
* *tid - t_ctid from the outdated tuple (ie, next updated version)
* priorXmax - t_xmax from the outdated tuple
+ * curCid - command ID of current command of my transaction
*
* *tid is also an output parameter: it's modified to hold the TID of the
* latest version of the tuple (note this may be changed even on failure)
***************
*** 1835,1841 ****
*/
TupleTableSlot *
EvalPlanQual(EState *estate, Index rti,
! ItemPointer tid, TransactionId priorXmax)
{
evalPlanQual *epq;
EState *epqstate;
--- 1839,1845 ----
*/
TupleTableSlot *
EvalPlanQual(EState *estate, Index rti,
! ItemPointer tid, TransactionId priorXmax, CommandId curCid)
{
evalPlanQual *epq;
EState *epqstate;
***************
*** 1912,1917 ****
--- 1916,1939 ----
}

/*
+ * If tuple was inserted by our own transaction, we have to check
+ * cmin against curCid: cmin >= curCid means our command cannot
+ * see the tuple, so we should ignore it. Without this we are
+ * open to the "Halloween problem" of indefinitely re-updating
+ * the same tuple. (We need not check cmax because
+ * HeapTupleSatisfiesDirty will consider a tuple deleted by
+ * our transaction dead, regardless of cmax.) We just checked
+ * that priorXmax == xmin, so we can test that variable instead
+ * of doing HeapTupleHeaderGetXmin again.
+ */
+ if (TransactionIdIsCurrentTransactionId(priorXmax) &&
+ HeapTupleHeaderGetCmin(tuple.t_data) >= curCid)
+ {
+ ReleaseBuffer(buffer);
+ return NULL;
+ }
+
+ /*
* We got tuple - now copy it for use by recheck query.
*/
copyTuple = heap_copytuple(&tuple);
Index: src/include/executor/executor.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/executor/executor.h,v
retrieving revision 1.120.2.1
diff -c -r1.120.2.1 executor.h
*** src/include/executor/executor.h 23 Nov 2005 20:28:05 -0000 1.120.2.1
--- src/include/executor/executor.h 12 Jan 2006 21:10:10 -0000
***************
*** 98,104 ****
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
! ItemPointer tid, TransactionId priorXmax);

/*
* prototypes from functions in execProcnode.c
--- 98,104 ----
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
! ItemPointer tid, TransactionId priorXmax, CommandId curCid);

/*
* prototypes from functions in execProcnode.c