Robert Haas escribió:

> The fact that there are no tests of this functionality is probably not
> a good thing.  We should add some.  At the moment, the following test
> case crashes, and it looks like this commit is responsible:
> 
> create table test_update2 (id integer);
> DECLARE test_update_cursor CURSOR FOR SELECT id, MIN(id) FROM
> test_update2 GROUP By id HAVING MIN(id) < 1 FOR UPDATE;

The attached patch fixes it, and I think it makes more sense than the
original coding to start with.

I will commit this tomorrow, adding a few test cases while at it.

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 9ff8050..49bd930 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1962,7 +1962,8 @@ preprocess_rowmarks(PlannerInfo *root)
 		 * CTIDs invalid.  This is also checked at parse time, but that's
 		 * insufficient because of rule substitution, query pullup, etc.
 		 */
-		CheckSelectLocking(parse);
+		CheckSelectLocking(parse, ((RowMarkClause *)
+							linitial(parse->rowMarks))->strength);
 	}
 	else
 	{
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 39036fb..a9d1fec 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2243,7 +2243,7 @@ LCS_asString(LockClauseStrength strength)
  * exported so planner can check again after rewriting, query pullup, etc
  */
 void
-CheckSelectLocking(Query *qry)
+CheckSelectLocking(Query *qry, LockClauseStrength strength)
 {
 	if (qry->setOperations)
 		ereport(ERROR,
@@ -2251,56 +2251,49 @@ CheckSelectLocking(Query *qry)
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with UNION/INTERSECT/EXCEPT",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 	if (qry->distinctClause != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with DISTINCT clause",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 	if (qry->groupClause != NIL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with GROUP BY clause",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 	if (qry->havingQual != NULL)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with HAVING clause",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 	if (qry->hasAggs)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with aggregate functions",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 	if (qry->hasWindowFuncs)
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with window functions",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 	if (expression_returns_set((Node *) qry->targetList))
 		ereport(ERROR,
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 /*------
 				   translator: %s is a SQL row locking clause such as FOR UPDATE */
 				 errmsg("%s is not allowed with set-returning functions in the target list",
-						LCS_asString(((RowMarkClause *)
-									  linitial(qry->rowMarks))->strength))));
+						LCS_asString(strength))));
 }
 
 /*
@@ -2321,7 +2314,7 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
 	Index		i;
 	LockingClause *allrels;
 
-	CheckSelectLocking(qry);
+	CheckSelectLocking(qry, lc->strength);
 
 	/* make a clause we can pass down to subqueries to select all rels */
 	allrels = makeNode(LockingClause);
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index b24b205..2fef2f7 100644
--- a/src/include/parser/analyze.h
+++ b/src/include/parser/analyze.h
@@ -37,7 +37,7 @@ extern Query *transformStmt(ParseState *pstate, Node *parseTree);
 extern bool analyze_requires_snapshot(Node *parseTree);
 
 extern char *LCS_asString(LockClauseStrength strength);
-extern void CheckSelectLocking(Query *qry);
+extern void CheckSelectLocking(Query *qry, LockClauseStrength strength);
 extern void applyLockingClause(Query *qry, Index rtindex,
 				   LockClauseStrength strength, bool noWait, bool pushedDown);
 
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to