From f593f623362709fdf97f5434ab996e8145870004 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 21 Apr 2008 03:49:45 +0000
Subject: [PATCH] Fix a couple of places in execMain that erroneously assumed
 that SELECT FOR 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 | 87 ++++++++++++++++++++-------------
 1 file changed, 52 insertions(+), 35 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index db69ebb140..8b14c04b56 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -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