Commit 88c55668 authored by Alvaro Herrera's avatar Alvaro Herrera

Fix crash in error report of invalid tuple lock

My tweak of these error messages in commit c359a1b0 contained the
thinko that a query would always have rowMarks set for a query
containing a locking clause.  Not so: when declaring a cursor, for
instance, rowMarks isn't set at the point we're checking, so we'd be
dereferencing a NULL pointer.

The fix is to pass the lock strength to the function raising the error,
instead of trying to reverse-engineer it.  The result not only is more
robust, but it also seems cleaner overall.

Per report from Robert Haas.
parent 05ee328d
...@@ -1962,7 +1962,8 @@ preprocess_rowmarks(PlannerInfo *root) ...@@ -1962,7 +1962,8 @@ preprocess_rowmarks(PlannerInfo *root)
* CTIDs invalid. This is also checked at parse time, but that's * CTIDs invalid. This is also checked at parse time, but that's
* insufficient because of rule substitution, query pullup, etc. * insufficient because of rule substitution, query pullup, etc.
*/ */
CheckSelectLocking(parse); CheckSelectLocking(parse, ((RowMarkClause *)
linitial(parse->rowMarks))->strength);
} }
else else
{ {
......
...@@ -2243,7 +2243,7 @@ LCS_asString(LockClauseStrength strength) ...@@ -2243,7 +2243,7 @@ LCS_asString(LockClauseStrength strength)
* exported so planner can check again after rewriting, query pullup, etc * exported so planner can check again after rewriting, query pullup, etc
*/ */
void void
CheckSelectLocking(Query *qry) CheckSelectLocking(Query *qry, LockClauseStrength strength)
{ {
if (qry->setOperations) if (qry->setOperations)
ereport(ERROR, ereport(ERROR,
...@@ -2251,56 +2251,49 @@ CheckSelectLocking(Query *qry) ...@@ -2251,56 +2251,49 @@ CheckSelectLocking(Query *qry)
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with UNION/INTERSECT/EXCEPT", errmsg("%s is not allowed with UNION/INTERSECT/EXCEPT",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
if (qry->distinctClause != NIL) if (qry->distinctClause != NIL)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with DISTINCT clause", errmsg("%s is not allowed with DISTINCT clause",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
if (qry->groupClause != NIL) if (qry->groupClause != NIL)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with GROUP BY clause", errmsg("%s is not allowed with GROUP BY clause",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
if (qry->havingQual != NULL) if (qry->havingQual != NULL)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with HAVING clause", errmsg("%s is not allowed with HAVING clause",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
if (qry->hasAggs) if (qry->hasAggs)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with aggregate functions", errmsg("%s is not allowed with aggregate functions",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
if (qry->hasWindowFuncs) if (qry->hasWindowFuncs)
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ translator: %s is a SQL row locking clause such as FOR UPDATE */
errmsg("%s is not allowed with window functions", errmsg("%s is not allowed with window functions",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
if (expression_returns_set((Node *) qry->targetList)) if (expression_returns_set((Node *) qry->targetList))
ereport(ERROR, ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/*------ /*------
translator: %s is a SQL row locking clause such as FOR UPDATE */ 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", errmsg("%s is not allowed with set-returning functions in the target list",
LCS_asString(((RowMarkClause *) LCS_asString(strength))));
linitial(qry->rowMarks))->strength))));
} }
/* /*
...@@ -2321,7 +2314,7 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc, ...@@ -2321,7 +2314,7 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
Index i; Index i;
LockingClause *allrels; LockingClause *allrels;
CheckSelectLocking(qry); CheckSelectLocking(qry, lc->strength);
/* make a clause we can pass down to subqueries to select all rels */ /* make a clause we can pass down to subqueries to select all rels */
allrels = makeNode(LockingClause); allrels = makeNode(LockingClause);
......
...@@ -37,7 +37,7 @@ extern Query *transformStmt(ParseState *pstate, Node *parseTree); ...@@ -37,7 +37,7 @@ extern Query *transformStmt(ParseState *pstate, Node *parseTree);
extern bool analyze_requires_snapshot(Node *parseTree); extern bool analyze_requires_snapshot(Node *parseTree);
extern char *LCS_asString(LockClauseStrength strength); 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, extern void applyLockingClause(Query *qry, Index rtindex,
LockClauseStrength strength, bool noWait, bool pushedDown); LockClauseStrength strength, bool noWait, bool pushedDown);
......
...@@ -292,6 +292,9 @@ NOTICE: materialized view "no_such_mv" does not exist, skipping ...@@ -292,6 +292,9 @@ NOTICE: materialized view "no_such_mv" does not exist, skipping
-- make sure invalid comination of options is prohibited -- make sure invalid comination of options is prohibited
REFRESH MATERIALIZED VIEW CONCURRENTLY tvmm WITH NO DATA; REFRESH MATERIALIZED VIEW CONCURRENTLY tvmm WITH NO DATA;
ERROR: CONCURRENTLY and WITH NO DATA options cannot be used together ERROR: CONCURRENTLY and WITH NO DATA options cannot be used together
-- no tuple locks on materialized views
SELECT * FROM tvvm FOR SHARE;
ERROR: cannot lock rows in materialized view "tvvm"
-- test join of mv and view -- test join of mv and view
SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM tm m LEFT JOIN tv v USING (type) ORDER BY type; SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM tm m LEFT JOIN tv v USING (type) ORDER BY type;
type | mtot | vtot type | mtot | vtot
......
...@@ -1226,6 +1226,10 @@ DECLARE c1 CURSOR FOR SELECT * FROM uctest; ...@@ -1226,6 +1226,10 @@ DECLARE c1 CURSOR FOR SELECT * FROM uctest;
DELETE FROM uctest WHERE CURRENT OF c1; -- fail, no current row DELETE FROM uctest WHERE CURRENT OF c1; -- fail, no current row
ERROR: cursor "c1" is not positioned on a row ERROR: cursor "c1" is not positioned on a row
ROLLBACK; ROLLBACK;
BEGIN;
DECLARE c1 CURSOR FOR SELECT MIN(f1) FROM uctest FOR UPDATE;
ERROR: FOR UPDATE is not allowed with aggregate functions
ROLLBACK;
-- WHERE CURRENT OF may someday work with views, but today is not that day. -- WHERE CURRENT OF may someday work with views, but today is not that day.
-- For now, just make sure it errors out cleanly. -- For now, just make sure it errors out cleanly.
CREATE TEMP VIEW ucview AS SELECT * FROM uctest; CREATE TEMP VIEW ucview AS SELECT * FROM uctest;
......
...@@ -317,6 +317,8 @@ SELECT q1 FROM int8_tbl EXCEPT ALL SELECT DISTINCT q2 FROM int8_tbl; ...@@ -317,6 +317,8 @@ SELECT q1 FROM int8_tbl EXCEPT ALL SELECT DISTINCT q2 FROM int8_tbl;
123 123
(3 rows) (3 rows)
SELECT q1 FROM int8_tbl EXCEPT ALL SELECT q1 FROM int8_tbl FOR NO KEY UPDATE;
ERROR: FOR NO KEY UPDATE is not allowed with UNION/INTERSECT/EXCEPT
-- --
-- Mixed types -- Mixed types
-- --
......
...@@ -95,6 +95,9 @@ DROP MATERIALIZED VIEW IF EXISTS no_such_mv; ...@@ -95,6 +95,9 @@ DROP MATERIALIZED VIEW IF EXISTS no_such_mv;
-- make sure invalid comination of options is prohibited -- make sure invalid comination of options is prohibited
REFRESH MATERIALIZED VIEW CONCURRENTLY tvmm WITH NO DATA; REFRESH MATERIALIZED VIEW CONCURRENTLY tvmm WITH NO DATA;
-- no tuple locks on materialized views
SELECT * FROM tvvm FOR SHARE;
-- test join of mv and view -- test join of mv and view
SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM tm m LEFT JOIN tv v USING (type) ORDER BY type; SELECT type, m.totamt AS mtot, v.totamt AS vtot FROM tm m LEFT JOIN tv v USING (type) ORDER BY type;
......
...@@ -447,6 +447,9 @@ BEGIN; ...@@ -447,6 +447,9 @@ BEGIN;
DECLARE c1 CURSOR FOR SELECT * FROM uctest; DECLARE c1 CURSOR FOR SELECT * FROM uctest;
DELETE FROM uctest WHERE CURRENT OF c1; -- fail, no current row DELETE FROM uctest WHERE CURRENT OF c1; -- fail, no current row
ROLLBACK; ROLLBACK;
BEGIN;
DECLARE c1 CURSOR FOR SELECT MIN(f1) FROM uctest FOR UPDATE;
ROLLBACK;
-- WHERE CURRENT OF may someday work with views, but today is not that day. -- WHERE CURRENT OF may someday work with views, but today is not that day.
-- For now, just make sure it errors out cleanly. -- For now, just make sure it errors out cleanly.
......
...@@ -109,6 +109,8 @@ SELECT q1 FROM int8_tbl EXCEPT ALL SELECT q2 FROM int8_tbl; ...@@ -109,6 +109,8 @@ SELECT q1 FROM int8_tbl EXCEPT ALL SELECT q2 FROM int8_tbl;
SELECT q1 FROM int8_tbl EXCEPT ALL SELECT DISTINCT q2 FROM int8_tbl; SELECT q1 FROM int8_tbl EXCEPT ALL SELECT DISTINCT q2 FROM int8_tbl;
SELECT q1 FROM int8_tbl EXCEPT ALL SELECT q1 FROM int8_tbl FOR NO KEY UPDATE;
-- --
-- Mixed types -- Mixed types
-- --
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment