Fix AcquireRewriteLocks to be sure that it acquires the right lock strength

when FOR UPDATE is propagated down into a sub-select expanded from a view.
Similar bug to parser's isLockedRel issue that I fixed yesterday; likewise
seems not quite worth the effort to back-patch.

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.190 2009/10/28 14:55:43 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.191 2009/10/28 17:36:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -55,7 +55,8 @@ static void markQueryForLocking(Query *qry, Node *jtnode,
 					bool forUpdate, bool noWait, bool pushedDown);
 static List *matchLocks(CmdType event, RuleLock *rulelocks,
 		   int varno, Query *parsetree);
-static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
+static Query *fireRIRrules(Query *parsetree, List *activeRIRs,
+						   bool forUpdatePushedDown);
 
 
 /*
@@ -64,6 +65,10 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
  *	  These locks will ensure that the relation schemas don't change under us
  *	  while we are rewriting and planning the query.
  *
+ * forUpdatePushedDown indicates that a pushed-down FOR UPDATE/SHARE applies
+ * to the current subquery, requiring all rels to be opened with RowShareLock.
+ * This should always be false at the start of the recursion.
+ *
  * A secondary purpose of this routine is to fix up JOIN RTE references to
  * dropped columns (see details below).  Because the RTEs are modified in
  * place, it is generally appropriate for the caller of this routine to have
@@ -91,7 +96,7 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
  * construction of a nested join was O(N^2) in the nesting depth.)
  */
 void
-AcquireRewriteLocks(Query *parsetree)
+AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
 {
 	ListCell   *l;
 	int			rt_index;
@@ -129,7 +134,8 @@ AcquireRewriteLocks(Query *parsetree)
 				 */
 				if (rt_index == parsetree->resultRelation)
 					lockmode = RowExclusiveLock;
-				else if (get_parse_rowmark(parsetree, rt_index) != NULL)
+				else if (forUpdatePushedDown ||
+						 get_parse_rowmark(parsetree, rt_index) != NULL)
 					lockmode = RowShareLock;
 				else
 					lockmode = AccessShareLock;
@@ -206,7 +212,9 @@ AcquireRewriteLocks(Query *parsetree)
 				 * The subquery RTE itself is all right, but we have to
 				 * recurse to process the represented subquery.
 				 */
-				AcquireRewriteLocks(rte->subquery);
+				AcquireRewriteLocks(rte->subquery,
+									(forUpdatePushedDown ||
+									 get_parse_rowmark(parsetree, rt_index) != NULL));
 				break;
 
 			default:
@@ -220,7 +228,7 @@ AcquireRewriteLocks(Query *parsetree)
 	{
 		CommonTableExpr *cte = (CommonTableExpr *) lfirst(l);
 
-		AcquireRewriteLocks((Query *) cte->ctequery);
+		AcquireRewriteLocks((Query *) cte->ctequery, false);
 	}
 
 	/*
@@ -245,7 +253,7 @@ acquireLocksOnSubLinks(Node *node, void *context)
 		SubLink    *sub = (SubLink *) node;
 
 		/* Do what we came for */
-		AcquireRewriteLocks((Query *) sub->subselect);
+		AcquireRewriteLocks((Query *) sub->subselect, false);
 		/* Fall through to process lefthand args of SubLink */
 	}
 
@@ -298,7 +306,7 @@ rewriteRuleAction(Query *parsetree,
 	/*
 	 * Acquire necessary locks and fix any deleted JOIN RTE entries.
 	 */
-	AcquireRewriteLocks(rule_action);
+	AcquireRewriteLocks(rule_action, false);
 	(void) acquireLocksOnSubLinks(rule_qual, NULL);
 
 	current_varno = rt_index;
@@ -1134,7 +1142,8 @@ ApplyRetrieveRule(Query *parsetree,
 				  int rt_index,
 				  bool relation_level,
 				  Relation relation,
-				  List *activeRIRs)
+				  List *activeRIRs,
+				  bool forUpdatePushedDown)
 {
 	Query	   *rule_action;
 	RangeTblEntry *rte,
@@ -1148,18 +1157,25 @@ ApplyRetrieveRule(Query *parsetree,
 	if (!relation_level)
 		elog(ERROR, "cannot handle per-attribute ON SELECT rule");
 
+	/*
+	 * If FOR UPDATE/SHARE of view, be sure we get right initial lock on the
+	 * relations it references.
+	 */
+	rc = get_parse_rowmark(parsetree, rt_index);
+	forUpdatePushedDown |= (rc != NULL);
+
 	/*
 	 * Make a modifiable copy of the view query, and acquire needed locks on
 	 * the relations it mentions.
 	 */
 	rule_action = copyObject(linitial(rule->actions));
 
-	AcquireRewriteLocks(rule_action);
+	AcquireRewriteLocks(rule_action, forUpdatePushedDown);
 
 	/*
 	 * Recursively expand any view references inside the view.
 	 */
-	rule_action = fireRIRrules(rule_action, activeRIRs);
+	rule_action = fireRIRrules(rule_action, activeRIRs, forUpdatePushedDown);
 
 	/*
 	 * VIEWs are really easy --- just plug the view query in as a subselect,
@@ -1192,8 +1208,11 @@ ApplyRetrieveRule(Query *parsetree,
 	 * 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.
+	 *
+	 * Note: we don't consider forUpdatePushedDown here; such marks will be
+	 * made by recursing from the upper level in markQueryForLocking.
 	 */
-	if ((rc = get_parse_rowmark(parsetree, rt_index)) != NULL)
+	if (rc != NULL)
 		markQueryForLocking(rule_action, (Node *) rule_action->jointree,
 							rc->forUpdate, rc->noWait, true);
 
@@ -1281,7 +1300,7 @@ fireRIRonSubLink(Node *node, List *activeRIRs)
 
 		/* Do what we came for */
 		sub->subselect = (Node *) fireRIRrules((Query *) sub->subselect,
-											   activeRIRs);
+											   activeRIRs, false);
 		/* Fall through to process lefthand args of SubLink */
 	}
 
@@ -1299,7 +1318,7 @@ fireRIRonSubLink(Node *node, List *activeRIRs)
  *	Apply all RIR rules on each rangetable entry in a query
  */
 static Query *
-fireRIRrules(Query *parsetree, List *activeRIRs)
+fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 {
 	int			rt_index;
 	ListCell   *lc;
@@ -1329,7 +1348,9 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
 		 */
 		if (rte->rtekind == RTE_SUBQUERY)
 		{
-			rte->subquery = fireRIRrules(rte->subquery, activeRIRs);
+			rte->subquery = fireRIRrules(rte->subquery, activeRIRs,
+										 (forUpdatePushedDown ||
+										  get_parse_rowmark(parsetree, rt_index) != NULL));
 			continue;
 		}
 
@@ -1406,7 +1427,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
 											  rt_index,
 											  rule->attrno == -1,
 											  rel,
-											  activeRIRs);
+											  activeRIRs,
+											  forUpdatePushedDown);
 			}
 
 			activeRIRs = list_delete_first(activeRIRs);
@@ -1421,7 +1443,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
 		CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc);
 
 		cte->ctequery = (Node *)
-			fireRIRrules((Query *) cte->ctequery, activeRIRs);
+			fireRIRrules((Query *) cte->ctequery, activeRIRs, false);
 	}
 
 	/*
@@ -1850,7 +1872,7 @@ QueryRewrite(Query *parsetree)
 	{
 		Query	   *query = (Query *) lfirst(l);
 
-		query = fireRIRrules(query, NIL);
+		query = fireRIRrules(query, NIL, false);
 
 		/*
 		 * If the query target was rewritten as a view, complain.

src/backend/utils/adt/ruleutils.c

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.311 2009/10/28 14:55:44 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.312 2009/10/28 17:36:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2175,7 +2175,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
 		query = getInsertSelectQuery(query, NULL);
 
 		/* Must acquire locks right away; see notes in get_query_def() */
-		AcquireRewriteLocks(query);
+		AcquireRewriteLocks(query, false);
 
 		context.buf = buf;
 		context.namespaces = list_make1(&dpns);
@@ -2320,7 +2320,7 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
 	 * consistent results.	Note we assume it's OK to scribble on the passed
 	 * querytree!
 	 */
-	AcquireRewriteLocks(query);
+	AcquireRewriteLocks(query, false);
 
 	context.buf = buf;
 	context.namespaces = lcons(&dpns, list_copy(parentnamespace));

src/include/rewrite/rewriteHandler.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/rewrite/rewriteHandler.h,v 1.31 2009/01/01 17:24:01 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/rewrite/rewriteHandler.h,v 1.32 2009/10/28 17:36:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,7 +18,7 @@
 #include "nodes/parsenodes.h"
 
 extern List *QueryRewrite(Query *parsetree);
-extern void AcquireRewriteLocks(Query *parsetree);
+extern void AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown);
 extern Node *build_column_default(Relation rel, int attrno);
 
 #endif   /* REWRITEHANDLER_H */