When FOR UPDATE/SHARE is used with LIMIT, put the LockRows plan node

underneath the Limit node, not atop it.  This fixes the old problem that such
a query might unexpectedly return fewer rows than the LIMIT says, due to
LockRows discarding updated rows.

There is a related problem that LockRows might destroy the sort ordering
produced by earlier steps; but fixing that by pushing LockRows below Sort
would create serious performance problems that are unjustified in many
real-world applications, as well as potential deadlock problems from locking
many more rows than expected.  Instead, keep the present semantics of applying
FOR UPDATE after ORDER BY within a single query level; but allow the user to
specify the other way by writing FOR UPDATE in a sub-select.  To make that
work, track whether FOR UPDATE appeared explicitly in sub-selects or got
pushed down from the parent, and don't flatten a sub-select that contained an
explicit FOR UPDATE.

12 files changed, 142 insertions, 85 deletions

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index deee9994170..a9efce40532 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.449 2009/10/26 02:26:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.450 2009/10/28 14:55:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1859,6 +1859,7 @@ _copyRowMarkClause(RowMarkClause *from)
 	COPY_SCALAR_FIELD(rti);
 	COPY_SCALAR_FIELD(forUpdate);
 	COPY_SCALAR_FIELD(noWait);
+	COPY_SCALAR_FIELD(pushedDown);
 
 	return newnode;
 }
@@ -2223,6 +2224,7 @@ _copyQuery(Query *from)
 	COPY_SCALAR_FIELD(hasSubLinks);
 	COPY_SCALAR_FIELD(hasDistinctOn);
 	COPY_SCALAR_FIELD(hasRecursive);
+	COPY_SCALAR_FIELD(hasForUpdate);
 	COPY_NODE_FIELD(cteList);
 	COPY_NODE_FIELD(rtable);
 	COPY_NODE_FIELD(jointree);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index f7e9547a1f8..d60d238be93 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.371 2009/10/26 02:26:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.372 2009/10/28 14:55:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -860,6 +860,7 @@ _equalQuery(Query *a, Query *b)
 	COMPARE_SCALAR_FIELD(hasSubLinks);
 	COMPARE_SCALAR_FIELD(hasDistinctOn);
 	COMPARE_SCALAR_FIELD(hasRecursive);
+	COMPARE_SCALAR_FIELD(hasForUpdate);
 	COMPARE_NODE_FIELD(cteList);
 	COMPARE_NODE_FIELD(rtable);
 	COMPARE_NODE_FIELD(jointree);
@@ -2198,6 +2199,7 @@ _equalRowMarkClause(RowMarkClause *a, RowMarkClause *b)
 	COMPARE_SCALAR_FIELD(rti);
 	COMPARE_SCALAR_FIELD(forUpdate);
 	COMPARE_SCALAR_FIELD(noWait);
+	COMPARE_SCALAR_FIELD(pushedDown);
 
 	return true;
 }
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ae7a859bd55..7abee7a2d84 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.370 2009/10/26 02:26:31 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.371 2009/10/28 14:55:38 tgl Exp $
  *
  * NOTES
  *	  Every node type that can appear in stored rules' parsetrees *must*
@@ -1987,6 +1987,7 @@ _outQuery(StringInfo str, Query *node)
 	WRITE_BOOL_FIELD(hasSubLinks);
 	WRITE_BOOL_FIELD(hasDistinctOn);
 	WRITE_BOOL_FIELD(hasRecursive);
+	WRITE_BOOL_FIELD(hasForUpdate);
 	WRITE_NODE_FIELD(cteList);
 	WRITE_NODE_FIELD(rtable);
 	WRITE_NODE_FIELD(jointree);
@@ -2036,6 +2037,7 @@ _outRowMarkClause(StringInfo str, RowMarkClause *node)
 	WRITE_UINT_FIELD(rti);
 	WRITE_BOOL_FIELD(forUpdate);
 	WRITE_BOOL_FIELD(noWait);
+	WRITE_BOOL_FIELD(pushedDown);
 }
 
 static void
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index a1520276e22..4192ca7e330 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.226 2009/10/26 02:26:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.227 2009/10/28 14:55:38 tgl Exp $
  *
  * NOTES
  *	  Path and Plan nodes do not have any readfuncs support, because we
@@ -203,6 +203,7 @@ _readQuery(void)
 	READ_BOOL_FIELD(hasSubLinks);
 	READ_BOOL_FIELD(hasDistinctOn);
 	READ_BOOL_FIELD(hasRecursive);
+	READ_BOOL_FIELD(hasForUpdate);
 	READ_NODE_FIELD(cteList);
 	READ_NODE_FIELD(rtable);
 	READ_NODE_FIELD(jointree);
@@ -295,6 +296,7 @@ _readRowMarkClause(void)
 	READ_UINT_FIELD(rti);
 	READ_BOOL_FIELD(forUpdate);
 	READ_BOOL_FIELD(noWait);
+	READ_BOOL_FIELD(pushedDown);
 
 	READ_DONE();
 }
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 0b396b29bcc..6b24098cdd3 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.260 2009/10/26 02:26:33 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.261 2009/10/28 14:55:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1638,19 +1638,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
 	}
 
 	/*
-	 * If there is a LIMIT/OFFSET clause, add the LIMIT node.
-	 */
-	if (parse->limitCount || parse->limitOffset)
-	{
-		result_plan = (Plan *) make_limit(result_plan,
-										  parse->limitOffset,
-										  parse->limitCount,
-										  offset_est,
-										  count_est);
-	}
-
-	/*
-	 * Finally, if there is a FOR UPDATE/SHARE clause, add the LockRows node.
+	 * If there is a FOR UPDATE/SHARE clause, add the LockRows node.
 	 * (Note: we intentionally test parse->rowMarks not root->rowMarks here.
 	 * If there are only non-locking rowmarks, they should be handled by
 	 * the ModifyTable node instead.)
@@ -1660,6 +1648,23 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
 		result_plan = (Plan *) make_lockrows(result_plan,
 											 root->rowMarks,
 											 SS_assign_special_param(root));
+		/*
+		 * The result can no longer be assumed sorted, since locking might
+		 * cause the sort key columns to be replaced with new values.
+		 */
+		current_pathkeys = NIL;
+	}
+
+	/*
+	 * Finally, if there is a LIMIT/OFFSET clause, add the LIMIT node.
+	 */
+	if (parse->limitCount || parse->limitOffset)
+	{
+		result_plan = (Plan *) make_limit(result_plan,
+										  parse->limitOffset,
+										  parse->limitCount,
+										  offset_est,
+										  count_est);
 	}
 
 	/* Compute result-relations list if needed */
@@ -1795,20 +1800,33 @@ preprocess_rowmarks(PlannerInfo *root)
 
 	/*
 	 * Convert RowMarkClauses to PlanRowMark representation.
-	 *
-	 * Note: currently, it is syntactically impossible to have FOR UPDATE
-	 * applied to an update/delete target rel.  If that ever becomes
-	 * possible, we should drop the target from the PlanRowMark list.
 	 */
 	prowmarks = NIL;
 	foreach(l, parse->rowMarks)
 	{
 		RowMarkClause *rc = (RowMarkClause *) lfirst(l);
-		PlanRowMark *newrc = makeNode(PlanRowMark);
+		RangeTblEntry *rte = rt_fetch(rc->rti, parse->rtable);
+		PlanRowMark *newrc;
 
+		/*
+		 * Currently, it is syntactically impossible to have FOR UPDATE
+		 * applied to an update/delete target rel.  If that ever becomes
+		 * possible, we should drop the target from the PlanRowMark list.
+		 */
 		Assert(rc->rti != parse->resultRelation);
+
+		/*
+		 * Ignore RowMarkClauses for subqueries; they aren't real tables
+		 * and can't support true locking.  Subqueries that got flattened
+		 * into the main query should be ignored completely.  Any that didn't
+		 * will get ROW_MARK_COPY items in the next loop.
+		 */
+		if (rte->rtekind != RTE_RELATION)
+			continue;
+
 		rels = bms_del_member(rels, rc->rti);
 
+		newrc = makeNode(PlanRowMark);
 		newrc->rti = newrc->prti = rc->rti;
 		if (rc->forUpdate)
 			newrc->markType = ROW_MARK_EXCLUSIVE;
@@ -1838,7 +1856,6 @@ preprocess_rowmarks(PlannerInfo *root)
 			continue;
 
 		newrc = makeNode(PlanRowMark);
-
 		newrc->rti = newrc->prti = i;
 		/* real tables support REFERENCE, anything else needs COPY */
 		if (rte->rtekind == RTE_RELATION)
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index f48bd31151c..7d4d7b217f7 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -16,7 +16,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.68 2009/10/26 02:26:35 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.69 2009/10/28 14:55:38 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1030,6 +1030,12 @@ is_simple_subquery(Query *subquery)
 	/*
 	 * Can't pull up a subquery involving grouping, aggregation, sorting,
 	 * limiting, or WITH.  (XXX WITH could possibly be allowed later)
+	 *
+	 * We also don't pull up a subquery that has explicit FOR UPDATE/SHARE
+	 * clauses, because pullup would cause the locking to occur semantically
+	 * higher than it should.  Implicit FOR UPDATE/SHARE is okay because
+	 * in that case the locking was originally declared in the upper query
+	 * anyway.
 	 */
 	if (subquery->hasAggs ||
 		subquery->hasWindowFuncs ||
@@ -1039,6 +1045,7 @@ is_simple_subquery(Query *subquery)
 		subquery->distinctClause ||
 		subquery->limitOffset ||
 		subquery->limitCount ||
+		subquery->hasForUpdate ||
 		subquery->cteList)
 		return false;
 
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 84cbd2142e3..c89c7c82a08 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -17,7 +17,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.394 2009/10/27 17:11:18 tgl Exp $
+ *	$PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.395 2009/10/28 14:55:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -60,8 +60,8 @@ static Query *transformDeclareCursorStmt(ParseState *pstate,
 						   DeclareCursorStmt *stmt);
 static Query *transformExplainStmt(ParseState *pstate,
 					 ExplainStmt *stmt);
-static void transformLockingClause(ParseState *pstate,
-					   Query *qry, LockingClause *lc);
+static void transformLockingClause(ParseState *pstate, Query *qry,
+								   LockingClause *lc, bool pushedDown);
 static bool check_parameter_resolution_walker(Node *node, ParseState *pstate);
 
 
@@ -896,7 +896,8 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 
 	foreach(l, stmt->lockingClause)
 	{
-		transformLockingClause(pstate, qry, (LockingClause *) lfirst(l));
+		transformLockingClause(pstate, qry,
+							   (LockingClause *) lfirst(l), false);
 	}
 
 	return qry;
@@ -1348,7 +1349,8 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 
 	foreach(l, lockingClause)
 	{
-		transformLockingClause(pstate, qry, (LockingClause *) lfirst(l));
+		transformLockingClause(pstate, qry,
+							   (LockingClause *) lfirst(l), false);
 	}
 
 	return qry;
@@ -2056,7 +2058,8 @@ CheckSelectLocking(Query *qry)
  * in rewriteHandler.c, and isLockedRefname() in parse_relation.c.
  */
 static void
-transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
+transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
+					   bool pushedDown)
 {
 	List	   *lockedRels = lc->lockedRels;
 	ListCell   *l;
@@ -2084,16 +2087,22 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
 			switch (rte->rtekind)
 			{
 				case RTE_RELATION:
-					applyLockingClause(qry, i, lc->forUpdate, lc->noWait);
+					applyLockingClause(qry, i,
+									   lc->forUpdate, lc->noWait, pushedDown);
 					rte->requiredPerms |= ACL_SELECT_FOR_UPDATE;
 					break;
 				case RTE_SUBQUERY:
-
+					applyLockingClause(qry, i,
+									   lc->forUpdate, lc->noWait, pushedDown);
 					/*
 					 * FOR UPDATE/SHARE of subquery is propagated to all of
-					 * subquery's rels
+					 * subquery's rels, too.  We could do this later (based
+					 * on the marking of the subquery RTE) but it is convenient
+					 * to have local knowledge in each query level about
+					 * which rels need to be opened with RowShareLock.
 					 */
-					transformLockingClause(pstate, rte->subquery, allrels);
+					transformLockingClause(pstate, rte->subquery,
+										   allrels, true);
 					break;
 				default:
 					/* ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE RTEs */
@@ -2127,16 +2136,17 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
 					{
 						case RTE_RELATION:
 							applyLockingClause(qry, i,
-											   lc->forUpdate, lc->noWait);
+											   lc->forUpdate, lc->noWait,
+											   pushedDown);
 							rte->requiredPerms |= ACL_SELECT_FOR_UPDATE;
 							break;
 						case RTE_SUBQUERY:
-
-							/*
-							 * FOR UPDATE/SHARE of subquery is propagated to
-							 * all of subquery's rels
-							 */
-							transformLockingClause(pstate, rte->subquery, allrels);
+							applyLockingClause(qry, i,
+											   lc->forUpdate, lc->noWait,
+											   pushedDown);
+							/* see comment above */
+							transformLockingClause(pstate, rte->subquery,
+												   allrels, true);
 							break;
 						case RTE_JOIN:
 							ereport(ERROR,
@@ -2190,10 +2200,15 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
  * Record locking info for a single rangetable item
  */
 void
-applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
+applyLockingClause(Query *qry, Index rtindex,
+				   bool forUpdate, bool noWait, bool pushedDown)
 {
 	RowMarkClause *rc;
 
+	/* If it's an explicit clause, make sure hasForUpdate gets set */
+	if (!pushedDown)
+		qry->hasForUpdate = true;
+
 	/* Check for pre-existing entry for same rtindex */
 	if ((rc = get_parse_rowmark(qry, rtindex)) != NULL)
 	{
@@ -2207,9 +2222,12 @@ applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
 		 * is a bit more debatable but raising an error doesn't seem helpful.
 		 * (Consider for instance SELECT FOR UPDATE NOWAIT from a view that
 		 * internally contains a plain FOR UPDATE spec.)
+		 *
+		 * And of course pushedDown becomes false if any clause is explicit.
 		 */
 		rc->forUpdate |= forUpdate;
 		rc->noWait |= noWait;
+		rc->pushedDown &= pushedDown;
 		return;
 	}
 
@@ -2218,6 +2236,7 @@ applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
 	rc->rti = rtindex;
 	rc->forUpdate = forUpdate;
 	rc->noWait = noWait;
+	rc->pushedDown = pushedDown;
 	qry->rowMarks = lappend(qry->rowMarks, rc);
 }
 
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 0ea80e5e8bc..8f5b3002278 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.189 2009/10/27 17:11:18 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.190 2009/10/28 14:55:43 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -52,7 +52,7 @@ static Node *get_assignment_input(Node *node);
 static void rewriteValuesRTE(RangeTblEntry *rte, Relation target_relation,
 				 List *attrnos);
 static void markQueryForLocking(Query *qry, Node *jtnode,
-					bool forUpdate, bool noWait);
+					bool forUpdate, bool noWait, bool pushedDown);
 static List *matchLocks(CmdType event, RuleLock *rulelocks,
 		   int varno, Query *parsetree);
 static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
@@ -1189,23 +1189,13 @@ ApplyRetrieveRule(Query *parsetree,
 	rte->modifiedCols = NULL;
 
 	/*
-	 * FOR UPDATE/SHARE of view?
+	 * If FOR UPDATE/SHARE of view, mark all the contained tables as
+	 * implicit FOR UPDATE/SHARE, the same as the parser would have done
+	 * if the view's subquery had been written out explicitly.
 	 */
 	if ((rc = get_parse_rowmark(parsetree, rt_index)) != NULL)
-	{
-		/*
-		 * Remove the view from the list of rels that will actually be marked
-		 * FOR UPDATE/SHARE by the executor.  It will still be access-checked
-		 * for write access, though.
-		 */
-		parsetree->rowMarks = list_delete_ptr(parsetree->rowMarks, rc);
-
-		/*
-		 * Set up the view's referenced tables as if FOR UPDATE/SHARE.
-		 */
 		markQueryForLocking(rule_action, (Node *) rule_action->jointree,
-							rc->forUpdate, rc->noWait);
-	}
+							rc->forUpdate, rc->noWait, true);
 
 	return parsetree;
 }
@@ -1222,7 +1212,8 @@ ApplyRetrieveRule(Query *parsetree,
  * to scan the jointree to determine which rels are used.
  */
 static void
-markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait)
+markQueryForLocking(Query *qry, Node *jtnode,
+					bool forUpdate, bool noWait, bool pushedDown)
 {
 	if (jtnode == NULL)
 		return;
@@ -1233,14 +1224,15 @@ markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait)
 
 		if (rte->rtekind == RTE_RELATION)
 		{
-			applyLockingClause(qry, rti, forUpdate, noWait);
+			applyLockingClause(qry, rti, forUpdate, noWait, pushedDown);
 			rte->requiredPerms |= ACL_SELECT_FOR_UPDATE;
 		}
 		else if (rte->rtekind == RTE_SUBQUERY)
 		{
+			applyLockingClause(qry, rti, forUpdate, noWait, pushedDown);
 			/* FOR UPDATE/SHARE of subquery is propagated to subquery's rels */
 			markQueryForLocking(rte->subquery, (Node *) rte->subquery->jointree,
-								forUpdate, noWait);
+								forUpdate, noWait, true);
 		}
 		/* other RTE types are unaffected by FOR UPDATE */
 	}
@@ -1250,14 +1242,14 @@ markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait)
 		ListCell   *l;
 
 		foreach(l, f->fromlist)
-			markQueryForLocking(qry, lfirst(l), forUpdate, noWait);
+			markQueryForLocking(qry, lfirst(l), forUpdate, noWait, pushedDown);
 	}
 	else if (IsA(jtnode, JoinExpr))
 	{
 		JoinExpr   *j = (JoinExpr *) jtnode;
 
-		markQueryForLocking(qry, j->larg, forUpdate, noWait);
-		markQueryForLocking(qry, j->rarg, forUpdate, noWait);
+		markQueryForLocking(qry, j->larg, forUpdate, noWait, pushedDown);
+		markQueryForLocking(qry, j->rarg, forUpdate, noWait, pushedDown);
 	}
 	else
 		elog(ERROR, "unrecognized node type: %d",
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8538c74d123..85e52293e76 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.310 2009/10/14 22:14:23 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.311 2009/10/28 14:55:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2549,21 +2549,28 @@ get_select_query_def(Query *query, deparse_context *context,
 	}
 
 	/* Add FOR UPDATE/SHARE clauses if present */
-	foreach(l, query->rowMarks)
+	if (query->hasForUpdate)
 	{
-		RowMarkClause *rc = (RowMarkClause *) lfirst(l);
-		RangeTblEntry *rte = rt_fetch(rc->rti, query->rtable);
+		foreach(l, query->rowMarks)
+		{
+			RowMarkClause *rc = (RowMarkClause *) lfirst(l);
+			RangeTblEntry *rte = rt_fetch(rc->rti, query->rtable);
 
-		if (rc->forUpdate)
-			appendContextKeyword(context, " FOR UPDATE",
-								 -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
-		else
-			appendContextKeyword(context, " FOR SHARE",
-								 -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
-		appendStringInfo(buf, " OF %s",
-						 quote_identifier(rte->eref->aliasname));
-		if (rc->noWait)
-			appendStringInfo(buf, " NOWAIT");
+			/* don't print implicit clauses */
+			if (rc->pushedDown)
+				continue;
+
+			if (rc->forUpdate)
+				appendContextKeyword(context, " FOR UPDATE",
+									 -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
+			else
+				appendContextKeyword(context, " FOR SHARE",
+									 -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
+			appendStringInfo(buf, " OF %s",
+							 quote_identifier(rte->eref->aliasname));
+			if (rc->noWait)
+				appendStringInfo(buf, " NOWAIT");
+		}
 	}
 
 	context->windowClause = save_windowclause;
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index aa9348a336f..74bf01d29c5 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -37,7 +37,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.547 2009/10/26 02:26:41 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.548 2009/10/28 14:55:44 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	200910251
+#define CATALOG_VERSION_NO	200910281
 
 #endif
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 450a89fe85b..2078526092b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.411 2009/10/26 02:26:41 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.412 2009/10/28 14:55:46 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -118,6 +118,7 @@ typedef struct Query
 	bool		hasSubLinks;	/* has subquery SubLink */
 	bool		hasDistinctOn;	/* distinctClause is from DISTINCT ON */
 	bool		hasRecursive;	/* WITH RECURSIVE was specified */
+	bool		hasForUpdate;	/* FOR UPDATE or FOR SHARE was specified */
 
 	List	   *cteList;		/* WITH list (of CommonTableExpr's) */
 
@@ -803,7 +804,12 @@ typedef struct WindowClause
  *	   parser output representation of FOR UPDATE/SHARE clauses
  *
  * Query.rowMarks contains a separate RowMarkClause node for each relation
- * identified as a FOR UPDATE/SHARE target.
+ * identified as a FOR UPDATE/SHARE target.  If FOR UPDATE/SHARE is applied
+ * to a subquery, we generate RowMarkClauses for all normal and subquery rels
+ * in the subquery, but they are marked pushedDown = true to distinguish them
+ * from clauses that were explicitly written at this query level.  Also,
+ * Query.hasForUpdate tells whether there were explicit FOR UPDATE/SHARE
+ * clauses in the current query level.
  */
 typedef struct RowMarkClause
 {
@@ -811,6 +817,7 @@ typedef struct RowMarkClause
 	Index		rti;			/* range table index of target relation */
 	bool		forUpdate;		/* true = FOR UPDATE, false = FOR SHARE */
 	bool		noWait;			/* NOWAIT option */
+	bool		pushedDown;		/* pushed down from higher query level? */
 } RowMarkClause;
 
 /*
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index 59e64c91455..8acd6153f8a 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.42 2009/10/27 17:11:18 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.43 2009/10/28 14:55:47 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -31,6 +31,6 @@ extern bool analyze_requires_snapshot(Node *parseTree);
 
 extern void CheckSelectLocking(Query *qry);
 extern void applyLockingClause(Query *qry, Index rtindex,
-				   bool forUpdate, bool noWait);
+				   bool forUpdate, bool noWait, bool pushedDown);
 
 #endif   /* ANALYZE_H */