Fix a couple of places in execMain that erroneously assumed that SELECT FOR
authorTom Lane
Mon, 21 Apr 2008 03:49:45 +0000 (03:49 +0000)
committerTom Lane
Mon, 21 Apr 2008 03:49:45 +0000 (03:49 +0000)
UPDATE/SHARE couldn't occur as a subquery in a query with a non-SELECT
top-level operation.  Symptoms included outright failure (as in report from
Mark Mielke) and silently neglecting to take the requested row locks.

Back-patch to 8.3, because the visible failure in the INSERT ... SELECT case
is a regression from 8.2.  I'm a bit hesitant to back-patch further given the
lack of field complaints.

src/backend/executor/execMain.c

index db69ebb14030b400d32a459df7fc54ef40bb1af3..8b14c04b56d0747358584af9b2c0559170378c44 100644 (file)
@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.305 2008/03/28 00:21:55 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.306 2008/04/21 03:49:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -754,6 +754,16 @@ InitPlan(QueryDesc *queryDesc, int eflags)
                 */
                estate->es_junkFilter =
                    estate->es_result_relation_info->ri_junkFilter;
+
+               /*
+                * We currently can't support rowmarks in this case, because
+                * the associated junk CTIDs might have different resnos in
+                * different subplans.
+                */
+               if (estate->es_rowMarks)
+                   ereport(ERROR,
+                           (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                            errmsg("SELECT FOR UPDATE/SHARE is not supported within a query with multiple result relations")));
            }
            else
            {
@@ -771,18 +781,6 @@ InitPlan(QueryDesc *queryDesc, int eflags)
                {
                    /* For SELECT, want to return the cleaned tuple type */
                    tupType = j->jf_cleanTupType;
-                   /* For SELECT FOR UPDATE/SHARE, find the ctid attrs now */
-                   foreach(l, estate->es_rowMarks)
-                   {
-                       ExecRowMark *erm = (ExecRowMark *) lfirst(l);
-                       char        resname[32];
-
-                       snprintf(resname, sizeof(resname), "ctid%u", erm->rti);
-                       erm->ctidAttNo = ExecFindJunkAttribute(j, resname);
-                       if (!AttributeNumberIsValid(erm->ctidAttNo))
-                           elog(ERROR, "could not find junk \"%s\" column",
-                                resname);
-                   }
                }
                else if (operation == CMD_UPDATE || operation == CMD_DELETE)
                {
@@ -791,10 +789,27 @@ InitPlan(QueryDesc *queryDesc, int eflags)
                    if (!AttributeNumberIsValid(j->jf_junkAttNo))
                        elog(ERROR, "could not find junk ctid column");
                }
+
+               /* For SELECT FOR UPDATE/SHARE, find the ctid attrs now */
+               foreach(l, estate->es_rowMarks)
+               {
+                   ExecRowMark *erm = (ExecRowMark *) lfirst(l);
+                   char        resname[32];
+
+                   snprintf(resname, sizeof(resname), "ctid%u", erm->rti);
+                   erm->ctidAttNo = ExecFindJunkAttribute(j, resname);
+                   if (!AttributeNumberIsValid(erm->ctidAttNo))
+                       elog(ERROR, "could not find junk \"%s\" column",
+                            resname);
+               }
            }
        }
        else
+       {
            estate->es_junkFilter = NULL;
+           if (estate->es_rowMarks)
+               elog(ERROR, "SELECT FOR UPDATE/SHARE, but no junk columns");
+       }
    }
 
    /*
@@ -1240,40 +1255,21 @@ lnext:  ;
        slot = planSlot;
 
        /*
-        * if we have a junk filter, then project a new tuple with the junk
+        * If we have a junk filter, then project a new tuple with the junk
         * removed.
         *
         * Store this new "clean" tuple in the junkfilter's resultSlot.
         * (Formerly, we stored it back over the "dirty" tuple, which is WRONG
         * because that tuple slot has the wrong descriptor.)
         *
-        * Also, extract all the junk information we need.
+        * But first, extract all the junk information we need.
         */
        if ((junkfilter = estate->es_junkFilter) != NULL)
        {
-           Datum       datum;
-           bool        isNull;
-
-           /*
-            * extract the 'ctid' junk attribute.
-            */
-           if (operation == CMD_UPDATE || operation == CMD_DELETE)
-           {
-               datum = ExecGetJunkAttribute(slot, junkfilter->jf_junkAttNo,
-                                            &isNull);
-               /* shouldn't ever get a null result... */
-               if (isNull)
-                   elog(ERROR, "ctid is NULL");
-
-               tupleid = (ItemPointer) DatumGetPointer(datum);
-               tuple_ctid = *tupleid;  /* make sure we don't free the ctid!! */
-               tupleid = &tuple_ctid;
-           }
-
            /*
             * Process any FOR UPDATE or FOR SHARE locking requested.
             */
-           else if (estate->es_rowMarks != NIL)
+           if (estate->es_rowMarks != NIL)
            {
                ListCell   *l;
 
@@ -1281,6 +1277,8 @@ lnext:    ;
                foreach(l, estate->es_rowMarks)
                {
                    ExecRowMark *erm = lfirst(l);
+                   Datum       datum;
+                   bool        isNull;
                    HeapTupleData tuple;
                    Buffer      buffer;
                    ItemPointerData update_ctid;
@@ -1352,6 +1350,25 @@ lnext:   ;
                }
            }
 
+           /*
+            * extract the 'ctid' junk attribute.
+            */
+           if (operation == CMD_UPDATE || operation == CMD_DELETE)
+           {
+               Datum       datum;
+               bool        isNull;
+
+               datum = ExecGetJunkAttribute(slot, junkfilter->jf_junkAttNo,
+                                            &isNull);
+               /* shouldn't ever get a null result... */
+               if (isNull)
+                   elog(ERROR, "ctid is NULL");
+
+               tupleid = (ItemPointer) DatumGetPointer(datum);
+               tuple_ctid = *tupleid;  /* make sure we don't free the ctid!! */
+               tupleid = &tuple_ctid;
+           }
+
            /*
             * Create a new "clean" tuple with all junk attributes removed. We
             * don't need to do this for DELETE, however (there will in fact