Fix a couple of places in execMain that erroneously assumed that SELECT FOR
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 21 Apr 2008 03:49:45 +0000 (03:49 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
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