Re: BUG #2166: attempted to update invisible tuple - Mailing list pgsql-bugs

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