Re: "ORDER BY" clause prevents "UPDATE WHERE CURRENT OF"

I wrote:
> [ dept of second thoughts... ]  Actually, given that he said FOR UPDATE,
> the plan should be propagating the tuple identity through to top level
> so that execMain can do its thing.  So in principle we could probably
> get the information without widespread changes.  This would fit
> reasonably well with the spec's requirements too --- any but trivial
> cursors are not deemed updatable unless you say FOR UPDATE.  But it
> would mean two completely independent implementations within
> execCurrent.c...

Here's a draft patch (no docs, no regression test) for that.  It doesn't
look as ugly as I expected.  Comments?  I'm hesitant to call this a bug
fix, and we are past feature freeze ...

            regards, tom lane

Index: src/backend/executor/execCurrent.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execCurrent.c,v
retrieving revision 1.7
diff -c -r1.7 execCurrent.c
*** src/backend/executor/execCurrent.c    12 May 2008 00:00:48 -0000    1.7
--- src/backend/executor/execCurrent.c    15 Nov 2008 00:04:17 -0000
***************
*** 46,51 ****
--- 46,53 ----
      char       *table_name;
      Portal        portal;
      QueryDesc  *queryDesc;
+     ExecRowMark *erm;
+     ListCell   *lc;
      ScanState  *scanstate;
      bool        lisnull;
      Oid            tuple_tableoid;
***************
*** 86,91 ****
--- 88,140 ----
                          cursor_name)));

      /*
+      * If the query uses FOR UPDATE, look through the executor's state for that
+      * to see if we can identify the target row that way.  We succeed if there
+      * is exactly one FOR UPDATE item for the requested table.  (Note:
+      * presently, FOR UPDATE is not allowed on inheritance trees, so there is
+      * no need to worry about whether a FOR UPDATE item is currently valid.)
+      */
+     erm = NULL;
+     foreach(lc, queryDesc->estate->es_rowMarks)
+     {
+         ExecRowMark *thiserm = (ExecRowMark *) lfirst(lc);
+
+         if (RelationGetRelid(thiserm->relation) == table_oid)
+         {
+             if (erm)
+             {
+                 /* multiple references to desired relation */
+                 erm = NULL;
+                 break;
+             }
+             erm = thiserm;
+         }
+     }
+
+     if (erm)
+     {
+         /*
+          * Okay, we were able to identify the target FOR UPDATE item.
+          *
+          * The cursor must have a current result row: per the SQL spec, it's
+          * an error if not.
+          */
+         if (portal->atStart || portal->atEnd)
+             ereport(ERROR,
+                     (errcode(ERRCODE_INVALID_CURSOR_STATE),
+                      errmsg("cursor \"%s\" is not positioned on a row",
+                             cursor_name)));
+         /* Return the currently scanned TID */
+         if (ItemPointerIsValid(&(erm->curCtid)))
+         {
+             *current_tid = erm->curCtid;
+             return true;
+         }
+         /* Inactive scan?  Probably can't happen at the moment */
+         return false;
+     }
+
+     /*
       * Dig through the cursor's plan to find the scan node.  Fail if it's not
       * there or buried underneath aggregation.
       */
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.315
diff -c -r1.315 execMain.c
*** src/backend/executor/execMain.c    6 Nov 2008 20:51:14 -0000    1.315
--- src/backend/executor/execMain.c    15 Nov 2008 00:04:17 -0000
***************
*** 602,607 ****
--- 602,608 ----
          erm->noWait = rc->noWait;
          /* We'll set up ctidAttno below */
          erm->ctidAttNo = InvalidAttrNumber;
+         ItemPointerSetInvalid(&(erm->curCtid));
          estate->es_rowMarks = lappend(estate->es_rowMarks, erm);
      }

***************
*** 1442,1447 ****
--- 1443,1451 ----
                              elog(ERROR, "unrecognized heap_lock_tuple status: %u",
                                   test);
                      }
+
+                     /* Remember tuple TID for WHERE CURRENT OF */
+                     erm->curCtid = tuple.t_self;
                  }
              }

Index: src/include/nodes/execnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/execnodes.h,v
retrieving revision 1.194
diff -c -r1.194 execnodes.h
*** src/include/nodes/execnodes.h    31 Oct 2008 19:37:56 -0000    1.194
--- src/include/nodes/execnodes.h    15 Nov 2008 00:04:17 -0000
***************
*** 376,381 ****
--- 376,382 ----
      bool        forUpdate;        /* true = FOR UPDATE, false = FOR SHARE */
      bool        noWait;            /* NOWAIT option */
      AttrNumber    ctidAttNo;        /* resno of its ctid junk attribute */
+     ItemPointerData curCtid;    /* ctid of currently locked tuple */
  } ExecRowMark;