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