diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml index 0c3e397805359628b18b3d0baabb7e76e6aeb6c8..c51bd767eba295ef62959613b787772f06eaedaf 100644 --- a/doc/src/sgml/ref/select.sgml +++ b/doc/src/sgml/ref/select.sgml @@ -1,5 +1,5 @@ @@ -1127,6 +1127,11 @@ FOR SHARE [ OF table_name [, ...] ] If FOR UPDATE or FOR SHARE is applied to a view or sub-query, it affects all tables used in the view or sub-query. + However, FOR UPDATE/FOR SHARE + do not apply to WITH queries referenced by the primary query. + If you want row locking to occur within a WITH query, specify + FOR UPDATE or FOR SHARE within the + WITH query. diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 5fb1b31688dd8f5377b02b2ea0ed74a4c87238be..84cbd2142e338991b3e2823c4da7c42e6c66fef4 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.393 2009/10/26 02:26:35 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.394 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -138,12 +138,14 @@ parse_analyze_varparams(Node *parseTree, const char *sourceText, */ Query * parse_sub_analyze(Node *parseTree, ParseState *parentParseState, - CommonTableExpr *parentCTE) + CommonTableExpr *parentCTE, + bool locked_from_parent) { ParseState *pstate = make_parsestate(parentParseState); Query *query; pstate->p_parent_cte = parentCTE; + pstate->p_locked_from_parent = locked_from_parent; query = transformStmt(pstate, parseTree); @@ -1424,7 +1426,7 @@ transformSetOperationTree(ParseState *pstate, SelectStmt *stmt, * of this sub-query, because they are not in the toplevel pstate's * namespace list. */ - selectQuery = parse_sub_analyze((Node *) stmt, pstate, NULL); + selectQuery = parse_sub_analyze((Node *) stmt, pstate, NULL, false); /* * Check for bogus references to Vars on the current query level (but @@ -2051,7 +2053,7 @@ CheckSelectLocking(Query *qry) * This basically involves replacing names by integer relids. * * NB: if you need to change this, see also markQueryForLocking() - * in rewriteHandler.c, and isLockedRel() in parse_relation.c. + * in rewriteHandler.c, and isLockedRefname() in parse_relation.c. */ static void transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc) @@ -2093,32 +2095,8 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc) */ transformLockingClause(pstate, rte->subquery, allrels); break; - case RTE_CTE: - { - /* - * We allow FOR UPDATE/SHARE of a WITH query to be - * propagated into the WITH, but it doesn't seem very - * sane to allow this for a reference to an - * outer-level WITH. And it definitely wouldn't work - * for a self-reference, since we're not done - * analyzing the CTE anyway. - */ - CommonTableExpr *cte; - - if (rte->ctelevelsup > 0 || rte->self_reference) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("SELECT FOR UPDATE/SHARE cannot be applied to an outer-level WITH query"))); - cte = GetCTEForRTE(pstate, rte, -1); - /* should be analyzed by now */ - Assert(IsA(cte->ctequery, Query)); - transformLockingClause(pstate, - (Query *) cte->ctequery, - allrels); - } - break; default: - /* ignore JOIN, SPECIAL, FUNCTION RTEs */ + /* ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE RTEs */ break; } } @@ -2185,30 +2163,10 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc) parser_errposition(pstate, thisrel->location))); break; case RTE_CTE: - { - /* - * We allow FOR UPDATE/SHARE of a WITH query - * to be propagated into the WITH, but it - * doesn't seem very sane to allow this for a - * reference to an outer-level WITH. And it - * definitely wouldn't work for a - * self-reference, since we're not done - * analyzing the CTE anyway. - */ - CommonTableExpr *cte; - - if (rte->ctelevelsup > 0 || rte->self_reference) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("SELECT FOR UPDATE/SHARE cannot be applied to an outer-level WITH query"), - parser_errposition(pstate, thisrel->location))); - cte = GetCTEForRTE(pstate, rte, -1); - /* should be analyzed by now */ - Assert(IsA(cte->ctequery, Query)); - transformLockingClause(pstate, - (Query *) cte->ctequery, - allrels); - } + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("SELECT FOR UPDATE/SHARE cannot be applied to a WITH query"), + parser_errposition(pstate, thisrel->location))); break; default: elog(ERROR, "unrecognized RTE type: %d", diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index c3bdf33a998d65c66cc906cd8fd180e0ba89a709..84443db8932dd6f18eef1e18e35ed0cf06b7762d 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.192 2009/09/09 03:32:52 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.193 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -479,7 +479,8 @@ transformRangeSubselect(ParseState *pstate, RangeSubselect *r) /* * Analyze and transform the subquery. */ - query = parse_sub_analyze(r->subquery, pstate, NULL); + query = parse_sub_analyze(r->subquery, pstate, NULL, + isLockedRefname(pstate, r->alias->aliasname)); /* * Check that we got something reasonable. Many of these conditions are diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c index d4a190b505ef5c8f24cb9d6308e6f898e6418b5e..4904d4b39f410c9a9f4b6e4c7c3f4d0ab40c58c2 100644 --- a/src/backend/parser/parse_cte.c +++ b/src/backend/parser/parse_cte.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_cte.c,v 2.7 2009/09/09 03:32:52 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_cte.c,v 2.8 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -229,7 +229,7 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte) /* Analysis not done already */ Assert(IsA(cte->ctequery, SelectStmt)); - query = parse_sub_analyze(cte->ctequery, pstate, cte); + query = parse_sub_analyze(cte->ctequery, pstate, cte, false); cte->ctequery = (Node *) query; /* diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index c6f3abe7caaaac9a1969a7ba0ce0e83b002472c5..2146329ad89693999ba2624e5cafdf6ad00e1be2 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.245 2009/10/21 20:22:38 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_expr.c,v 1.246 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1259,7 +1259,7 @@ transformSubLink(ParseState *pstate, SubLink *sublink) return result; pstate->p_hasSubLinks = true; - qtree = parse_sub_analyze(sublink->subselect, pstate, NULL); + qtree = parse_sub_analyze(sublink->subselect, pstate, NULL, false); /* * Check that we got something reasonable. Many of these conditions are diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 1a5f77d272d7c288a11e37f47ac4bdc1af333164..9b35577ec3c8e2606b398495880b566ec7ecce83 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_relation.c,v 1.145 2009/10/26 02:26:35 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_relation.c,v 1.146 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -38,7 +38,6 @@ static RangeTblEntry *scanNameSpaceForRelid(ParseState *pstate, Oid relid, int location); static void markRTEForSelectPriv(ParseState *pstate, RangeTblEntry *rte, int rtindex, AttrNumber col); -static bool isLockedRel(ParseState *pstate, char *refname); static void expandRelation(Oid relid, Alias *eref, int rtindex, int sublevels_up, int location, bool include_dropped, @@ -909,7 +908,7 @@ addRangeTableEntry(ParseState *pstate, * to a rel in a statement, be careful to get the right access level * depending on whether we're doing SELECT FOR UPDATE/SHARE. */ - lockmode = isLockedRel(pstate, refname) ? RowShareLock : AccessShareLock; + lockmode = isLockedRefname(pstate, refname) ? RowShareLock : AccessShareLock; rel = parserOpenTable(pstate, relation, lockmode); rte->relid = RelationGetRelid(rel); @@ -1454,40 +1453,46 @@ addRangeTableEntryForCTE(ParseState *pstate, /* * Has the specified refname been selected FOR UPDATE/FOR SHARE? * - * Note: we pay no attention to whether it's FOR UPDATE vs FOR SHARE. + * This is used when we have not yet done transformLockingClause, but need + * to know the correct lock to take during initial opening of relations. + * + * Note: we pay no attention to whether it's FOR UPDATE vs FOR SHARE, + * since the table-level lock is the same either way. */ -static bool -isLockedRel(ParseState *pstate, char *refname) +bool +isLockedRefname(ParseState *pstate, const char *refname) { - /* Outer loop to check parent query levels as well as this one */ - while (pstate != NULL) + ListCell *l; + + /* + * If we are in a subquery specified as locked FOR UPDATE/SHARE from + * parent level, then act as though there's a generic FOR UPDATE here. + */ + if (pstate->p_locked_from_parent) + return true; + + foreach(l, pstate->p_locking_clause) { - ListCell *l; + LockingClause *lc = (LockingClause *) lfirst(l); - foreach(l, pstate->p_locking_clause) + if (lc->lockedRels == NIL) { - LockingClause *lc = (LockingClause *) lfirst(l); + /* all tables used in query */ + return true; + } + else + { + /* just the named tables */ + ListCell *l2; - if (lc->lockedRels == NIL) - { - /* all tables used in query */ - return true; - } - else + foreach(l2, lc->lockedRels) { - /* just the named tables */ - ListCell *l2; - - foreach(l2, lc->lockedRels) - { - RangeVar *thisrel = (RangeVar *) lfirst(l2); + RangeVar *thisrel = (RangeVar *) lfirst(l2); - if (strcmp(refname, thisrel->relname) == 0) - return true; - } + if (strcmp(refname, thisrel->relname) == 0) + return true; } } - pstate = pstate->parentParseState; } return false; } diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index a4d9ae556047caecc6deaba57b49a1aaad886694..0ea80e5e8bc80ce94a1fc045ed86b8fd23a65358 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.188 2009/10/26 02:26:36 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.189 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1242,35 +1242,7 @@ markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait) markQueryForLocking(rte->subquery, (Node *) rte->subquery->jointree, forUpdate, noWait); } - else if (rte->rtekind == RTE_CTE) - { - /* - * We allow FOR UPDATE/SHARE of a WITH query to be propagated into - * the WITH, but it doesn't seem very sane to allow this for a - * reference to an outer-level WITH (compare - * transformLockingClause). Which simplifies life here. - */ - CommonTableExpr *cte = NULL; - ListCell *lc; - - if (rte->ctelevelsup > 0 || rte->self_reference) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("SELECT FOR UPDATE/SHARE cannot be applied to an outer-level WITH query"))); - foreach(lc, qry->cteList) - { - cte = (CommonTableExpr *) lfirst(lc); - if (strcmp(cte->ctename, rte->ctename) == 0) - break; - } - if (lc == NULL) /* shouldn't happen */ - elog(ERROR, "could not find CTE \"%s\"", rte->ctename); - /* should be analyzed by now */ - Assert(IsA(cte->ctequery, Query)); - markQueryForLocking((Query *) cte->ctequery, - (Node *) ((Query *) cte->ctequery)->jointree, - forUpdate, noWait); - } + /* other RTE types are unaffected by FOR UPDATE */ } else if (IsA(jtnode, FromExpr)) { diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h index 41282675d4c49a3672d1b35d37029e6a98ed7716..59e64c91455f75485c55716e242d59b30a09246c 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.41 2009/09/09 03:32:52 tgl Exp $ + * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.42 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -23,7 +23,8 @@ extern Query *parse_analyze_varparams(Node *parseTree, const char *sourceText, Oid **paramTypes, int *numParams); extern Query *parse_sub_analyze(Node *parseTree, ParseState *parentParseState, - CommonTableExpr *parentCTE); + CommonTableExpr *parentCTE, + bool locked_from_parent); extern Query *transformStmt(ParseState *pstate, Node *parseTree); extern bool analyze_requires_snapshot(Node *parseTree); diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 110ca3fc4a31dd07868432e9df5295312a491cb6..5eb446f12042b695b16f1dd8afec56126b0215b0 100644 --- a/src/include/parser/parse_node.h +++ b/src/include/parser/parse_node.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/parse_node.h,v 1.64 2009/10/21 20:22:38 tgl Exp $ + * $PostgreSQL: pgsql/src/include/parser/parse_node.h,v 1.65 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -103,6 +103,7 @@ typedef struct ParseState bool p_hasSubLinks; bool p_is_insert; bool p_is_update; + bool p_locked_from_parent; Relation p_target_relation; RangeTblEntry *p_target_rangetblentry; } ParseState; diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index 69a914376c7f21d5f2b9653554b73667d82369f4..96b60de480e6ecea2b4bc3c436f02e9dcc547ed3 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.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/parse_relation.h,v 1.65 2009/10/21 20:22:38 tgl Exp $ + * $PostgreSQL: pgsql/src/include/parser/parse_relation.h,v 1.66 2009/10/27 17:11:18 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -81,6 +81,7 @@ extern RangeTblEntry *addRangeTableEntryForCTE(ParseState *pstate, Index levelsup, Alias *alias, bool inFromCl); +extern bool isLockedRefname(ParseState *pstate, const char *refname); extern void addRTEtoQuery(ParseState *pstate, RangeTblEntry *rte, bool addToJoinList, bool addToRelNameSpace, bool addToVarNameSpace);