From 46e3a16b050a23b924e5d8a75c8bb7068c26aa96 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 28 Oct 2009 14:55:47 +0000
Subject: [PATCH] When FOR UPDATE/SHARE is used with LIMIT, put the LockRows
plan node underneath the Limit node, not atop it. This fixes the old problem
that such a query might unexpectedly return fewer rows than the LIMIT says,
due to LockRows discarding updated rows.
There is a related problem that LockRows might destroy the sort ordering
produced by earlier steps; but fixing that by pushing LockRows below Sort
would create serious performance problems that are unjustified in many
real-world applications, as well as potential deadlock problems from locking
many more rows than expected. Instead, keep the present semantics of applying
FOR UPDATE after ORDER BY within a single query level; but allow the user to
specify the other way by writing FOR UPDATE in a sub-select. To make that
work, track whether FOR UPDATE appeared explicitly in sub-selects or got
pushed down from the parent, and don't flatten a sub-select that contained an
explicit FOR UPDATE.
---
doc/src/sgml/ref/select.sgml | 124 +++++++++++++++-------
src/backend/nodes/copyfuncs.c | 4 +-
src/backend/nodes/equalfuncs.c | 4 +-
src/backend/nodes/outfuncs.c | 4 +-
src/backend/nodes/readfuncs.c | 4 +-
src/backend/optimizer/plan/planner.c | 57 ++++++----
src/backend/optimizer/prep/prepjointree.c | 9 +-
src/backend/parser/analyze.c | 55 ++++++----
src/backend/rewrite/rewriteHandler.c | 36 +++----
src/backend/utils/adt/ruleutils.c | 35 +++---
src/include/catalog/catversion.h | 4 +-
src/include/nodes/parsenodes.h | 11 +-
src/include/parser/analyze.h | 4 +-
13 files changed, 225 insertions(+), 126 deletions(-)
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index c51bd767eba..804a697e496 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -1,5 +1,5 @@
<!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/select.sgml,v 1.127 2009/10/27 17:11:18 tgl Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/select.sgml,v 1.128 2009/10/28 14:55:37 tgl Exp $
PostgreSQL documentation
-->
@@ -1092,22 +1092,12 @@ FOR SHARE [ OF <replaceable class="parameter">table_name</replaceable> [, ...] ]
has already locked a selected row or rows, <command>SELECT FOR
UPDATE</command> will wait for the other transaction to complete,
and will then lock and return the updated row (or no row, if the
- row was deleted). For further discussion see <xref
+ row was deleted). Within a <literal>SERIALIZABLE</> transaction,
+ however, an error will be thrown if a row to be locked has changed
+ since the transaction started. For further discussion see <xref
linkend="mvcc">.
</para>
- <para>
- To prevent the operation from waiting for other transactions to commit,
- use the <literal>NOWAIT</> option. <command>SELECT FOR UPDATE
- NOWAIT</command> reports an error, rather than waiting, if a selected row
- cannot be locked immediately. Note that <literal>NOWAIT</> applies only
- to the row-level lock(s) — the required <literal>ROW SHARE</literal>
- table-level lock is still taken in the ordinary way (see
- <xref linkend="mvcc">). You can use the <literal>NOWAIT</> option of
- <xref linkend="sql-lock" endterm="sql-lock-title">
- if you need to acquire the table-level lock without waiting.
- </para>
-
<para>
<literal>FOR SHARE</literal> behaves similarly, except that it
acquires a shared rather than exclusive lock on each retrieved
@@ -1117,13 +1107,26 @@ FOR SHARE [ OF <replaceable class="parameter">table_name</replaceable> [, ...] ]
from performing <command>SELECT FOR SHARE</command>.
</para>
+ <para>
+ To prevent the operation from waiting for other transactions to commit,
+ use the <literal>NOWAIT</> option. With <literal>NOWAIT</>, the statement
+ reports an error, rather than waiting, if a selected row
+ cannot be locked immediately. Note that <literal>NOWAIT</> applies only
+ to the row-level lock(s) — the required <literal>ROW SHARE</literal>
+ table-level lock is still taken in the ordinary way (see
+ <xref linkend="mvcc">). You can use
+ <xref linkend="sql-lock" endterm="sql-lock-title">
+ with the <literal>NOWAIT</> option first,
+ if you need to acquire the table-level lock without waiting.
+ </para>
+
<para>
If specific tables are named in <literal>FOR UPDATE</literal>
or <literal>FOR SHARE</literal>,
then only rows coming from those tables are locked; any other
tables used in the <command>SELECT</command> are simply read as
usual. A <literal>FOR UPDATE</literal> or <literal>FOR SHARE</literal>
- clause without a table list affects all tables used in the command.
+ clause without a table list affects all tables used in the statement.
If <literal>FOR UPDATE</literal> or <literal>FOR SHARE</literal> is
applied to a view or sub-query, it affects all tables used in
the view or sub-query.
@@ -1151,6 +1154,36 @@ FOR SHARE [ OF <replaceable class="parameter">table_name</replaceable> [, ...] ]
individual table rows; for example they cannot be used with aggregation.
</para>
+ <para>
+ When <literal>FOR UPDATE</literal> or <literal>FOR SHARE</literal>
+ appears at the top level of a <command>SELECT</> query, the rows that
+ are locked are exactly those that are returned by the query; in the
+ case of a join query, the rows locked are those that contribute to
+ returned join rows. In addition, rows that satisfied the query
+ conditions as of the query snapshot will be locked, although they
+ will not be returned if they have since been updated to not satisfy
+ the query conditions. If a <literal>LIMIT</> is used, locking stops
+ once enough rows have been returned to satisfy the limit (but note that
+ rows skipped over by <literal>OFFSET</> will get locked). Similarly,
+ if <literal>FOR UPDATE</literal> or <literal>FOR SHARE</literal>
+ is used in a cursor's query, only rows actually fetched or stepped past
+ by the cursor will be locked.
+ </para>
+
+ <para>
+ When <literal>FOR UPDATE</literal> or <literal>FOR SHARE</literal>
+ appears in a sub-<command>SELECT</>, the rows locked are those
+ returned to the outer query by the sub-query. This might involve
+ fewer rows than inspection of the sub-query alone would suggest,
+ since conditions from the outer query might be used to optimize
+ execution of the sub-query. For example,
+<programlisting>
+SELECT * FROM (SELECT * FROM mytable FOR UPDATE) ss WHERE col1 = 5;
+</programlisting>
+ will lock only rows having <literal>col1 = 5</>, even though that
+ condition is not textually within the sub-query.
+ </para>
+
<caution>
<para>
Avoid locking a row and then modifying it within a later savepoint or
@@ -1177,30 +1210,26 @@ ROLLBACK TO s;
<caution>
<para>
- It is possible for a <command>SELECT</> command using both
- <literal>LIMIT</literal> and <literal>FOR UPDATE/SHARE</literal>
- clauses to return fewer rows than specified by <literal>LIMIT</literal>.
- This is because <literal>LIMIT</> is applied first. The command
- selects the specified number of rows,
- but might then block trying to obtain a lock on one or more of them.
- Once the <literal>SELECT</> unblocks, the row might have been deleted
- or updated so that it does not meet the query <literal>WHERE</> condition
- anymore, in which case it will not be returned.
- </para>
- </caution>
-
- <caution>
- <para>
- Similarly, it is possible for a <command>SELECT</> command
- using <literal>ORDER BY</literal> and <literal>FOR
- UPDATE/SHARE</literal> to return rows out of order. This is
- because <literal>ORDER BY</> is applied first. The command
- orders the result, but might then block trying to obtain a lock
- on one or more of the rows. Once the <literal>SELECT</>
- unblocks, one of the ordered columns might have been modified
- and be returned out of order. A workaround is to perform
- <command>SELECT ... FOR UPDATE/SHARE</> and then <command>SELECT
- ... ORDER BY</>.
+ It is possible for a <command>SELECT</> command using <literal>ORDER
+ BY</literal> and <literal>FOR UPDATE/SHARE</literal> to return rows out of
+ order. This is because <literal>ORDER BY</> is applied first.
+ The command sorts the result, but might then block trying to obtain a lock
+ on one or more of the rows. Once the <literal>SELECT</> unblocks, some
+ of the ordering column values might have been modified, leading to those
+ rows appearing to be out of order (though they are in order in terms
+ of the original column values). This can be worked around at need by
+ placing the <literal>FOR UPDATE/SHARE</literal> clause in a sub-query,
+ for example
+<programlisting>
+SELECT * FROM (SELECT * FROM mytable FOR UPDATE) ss ORDER BY column1;
+</programlisting>
+ Note that this will result in locking all rows of <structname>mytable</>,
+ whereas <literal>FOR UPDATE</> at the top level would lock only the
+ actually returned rows. This can make for a significant performance
+ difference, particularly if the <literal>ORDER BY</> is combined with
+ <literal>LIMIT</> or other restrictions. So this technique is recommended
+ only if concurrent updates of the ordering columns are expected and a
+ strictly sorted result is required.
</para>
</caution>
</refsect2>
@@ -1541,15 +1570,28 @@ SELECT distributors.* WHERE distributors.name = 'Westward';
used by <productname>MySQL</productname>. The SQL:2008 standard
has introduced the clauses <literal>OFFSET ... FETCH {FIRST|NEXT}
...</literal> for the same functionality, as shown above
- in <xref linkend="sql-limit" endterm="sql-limit-title">, and this
+ in <xref linkend="sql-limit" endterm="sql-limit-title">. This
syntax is also used by <productname>IBM DB2</productname>.
(Applications written for <productname>Oracle</productname>
frequently use a workaround involving the automatically
- generated <literal>rownum</literal> column, not available in
+ generated <literal>rownum</literal> column, which is not available in
PostgreSQL, to implement the effects of these clauses.)
</para>
</refsect2>
+ <refsect2>
+ <title><literal>FOR UPDATE</> and <literal>FOR SHARE</></title>
+
+ <para>
+ Although <literal>FOR UPDATE</> appears in the SQL standard, the
+ standard allows it only as an option of <command>DECLARE CURSOR</>.
+ <productname>PostgreSQL</productname> allows it in any <command>SELECT</>
+ query as well as in sub-<command>SELECT</>s, but this is an extension.
+ The <literal>FOR SHARE</> variant, and the <literal>NOWAIT</> option,
+ do not appear in the standard.
+ </para>
+ </refsect2>
+
<refsect2>
<title>Nonstandard Clauses</title>
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index deee9994170..a9efce40532 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -15,7 +15,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.449 2009/10/26 02:26:31 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.450 2009/10/28 14:55:38 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1859,6 +1859,7 @@ _copyRowMarkClause(RowMarkClause *from)
COPY_SCALAR_FIELD(rti);
COPY_SCALAR_FIELD(forUpdate);
COPY_SCALAR_FIELD(noWait);
+ COPY_SCALAR_FIELD(pushedDown);
return newnode;
}
@@ -2223,6 +2224,7 @@ _copyQuery(Query *from)
COPY_SCALAR_FIELD(hasSubLinks);
COPY_SCALAR_FIELD(hasDistinctOn);
COPY_SCALAR_FIELD(hasRecursive);
+ COPY_SCALAR_FIELD(hasForUpdate);
COPY_NODE_FIELD(cteList);
COPY_NODE_FIELD(rtable);
COPY_NODE_FIELD(jointree);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index f7e9547a1f8..d60d238be93 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -22,7 +22,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.371 2009/10/26 02:26:31 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.372 2009/10/28 14:55:38 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -860,6 +860,7 @@ _equalQuery(Query *a, Query *b)
COMPARE_SCALAR_FIELD(hasSubLinks);
COMPARE_SCALAR_FIELD(hasDistinctOn);
COMPARE_SCALAR_FIELD(hasRecursive);
+ COMPARE_SCALAR_FIELD(hasForUpdate);
COMPARE_NODE_FIELD(cteList);
COMPARE_NODE_FIELD(rtable);
COMPARE_NODE_FIELD(jointree);
@@ -2198,6 +2199,7 @@ _equalRowMarkClause(RowMarkClause *a, RowMarkClause *b)
COMPARE_SCALAR_FIELD(rti);
COMPARE_SCALAR_FIELD(forUpdate);
COMPARE_SCALAR_FIELD(noWait);
+ COMPARE_SCALAR_FIELD(pushedDown);
return true;
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index ae7a859bd55..7abee7a2d84 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.370 2009/10/26 02:26:31 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/outfuncs.c,v 1.371 2009/10/28 14:55:38 tgl Exp $
*
* NOTES
* Every node type that can appear in stored rules' parsetrees *must*
@@ -1987,6 +1987,7 @@ _outQuery(StringInfo str, Query *node)
WRITE_BOOL_FIELD(hasSubLinks);
WRITE_BOOL_FIELD(hasDistinctOn);
WRITE_BOOL_FIELD(hasRecursive);
+ WRITE_BOOL_FIELD(hasForUpdate);
WRITE_NODE_FIELD(cteList);
WRITE_NODE_FIELD(rtable);
WRITE_NODE_FIELD(jointree);
@@ -2036,6 +2037,7 @@ _outRowMarkClause(StringInfo str, RowMarkClause *node)
WRITE_UINT_FIELD(rti);
WRITE_BOOL_FIELD(forUpdate);
WRITE_BOOL_FIELD(noWait);
+ WRITE_BOOL_FIELD(pushedDown);
}
static void
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index a1520276e22..4192ca7e330 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.226 2009/10/26 02:26:32 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/nodes/readfuncs.c,v 1.227 2009/10/28 14:55:38 tgl Exp $
*
* NOTES
* Path and Plan nodes do not have any readfuncs support, because we
@@ -203,6 +203,7 @@ _readQuery(void)
READ_BOOL_FIELD(hasSubLinks);
READ_BOOL_FIELD(hasDistinctOn);
READ_BOOL_FIELD(hasRecursive);
+ READ_BOOL_FIELD(hasForUpdate);
READ_NODE_FIELD(cteList);
READ_NODE_FIELD(rtable);
READ_NODE_FIELD(jointree);
@@ -295,6 +296,7 @@ _readRowMarkClause(void)
READ_UINT_FIELD(rti);
READ_BOOL_FIELD(forUpdate);
READ_BOOL_FIELD(noWait);
+ READ_BOOL_FIELD(pushedDown);
READ_DONE();
}
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 0b396b29bcc..6b24098cdd3 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.260 2009/10/26 02:26:33 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.261 2009/10/28 14:55:38 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1638,19 +1638,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
}
/*
- * If there is a LIMIT/OFFSET clause, add the LIMIT node.
- */
- if (parse->limitCount || parse->limitOffset)
- {
- result_plan = (Plan *) make_limit(result_plan,
- parse->limitOffset,
- parse->limitCount,
- offset_est,
- count_est);
- }
-
- /*
- * Finally, if there is a FOR UPDATE/SHARE clause, add the LockRows node.
+ * If there is a FOR UPDATE/SHARE clause, add the LockRows node.
* (Note: we intentionally test parse->rowMarks not root->rowMarks here.
* If there are only non-locking rowmarks, they should be handled by
* the ModifyTable node instead.)
@@ -1660,6 +1648,23 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
result_plan = (Plan *) make_lockrows(result_plan,
root->rowMarks,
SS_assign_special_param(root));
+ /*
+ * The result can no longer be assumed sorted, since locking might
+ * cause the sort key columns to be replaced with new values.
+ */
+ current_pathkeys = NIL;
+ }
+
+ /*
+ * Finally, if there is a LIMIT/OFFSET clause, add the LIMIT node.
+ */
+ if (parse->limitCount || parse->limitOffset)
+ {
+ result_plan = (Plan *) make_limit(result_plan,
+ parse->limitOffset,
+ parse->limitCount,
+ offset_est,
+ count_est);
}
/* Compute result-relations list if needed */
@@ -1795,20 +1800,33 @@ preprocess_rowmarks(PlannerInfo *root)
/*
* Convert RowMarkClauses to PlanRowMark representation.
- *
- * Note: currently, it is syntactically impossible to have FOR UPDATE
- * applied to an update/delete target rel. If that ever becomes
- * possible, we should drop the target from the PlanRowMark list.
*/
prowmarks = NIL;
foreach(l, parse->rowMarks)
{
RowMarkClause *rc = (RowMarkClause *) lfirst(l);
- PlanRowMark *newrc = makeNode(PlanRowMark);
+ RangeTblEntry *rte = rt_fetch(rc->rti, parse->rtable);
+ PlanRowMark *newrc;
+ /*
+ * Currently, it is syntactically impossible to have FOR UPDATE
+ * applied to an update/delete target rel. If that ever becomes
+ * possible, we should drop the target from the PlanRowMark list.
+ */
Assert(rc->rti != parse->resultRelation);
+
+ /*
+ * Ignore RowMarkClauses for subqueries; they aren't real tables
+ * and can't support true locking. Subqueries that got flattened
+ * into the main query should be ignored completely. Any that didn't
+ * will get ROW_MARK_COPY items in the next loop.
+ */
+ if (rte->rtekind != RTE_RELATION)
+ continue;
+
rels = bms_del_member(rels, rc->rti);
+ newrc = makeNode(PlanRowMark);
newrc->rti = newrc->prti = rc->rti;
if (rc->forUpdate)
newrc->markType = ROW_MARK_EXCLUSIVE;
@@ -1838,7 +1856,6 @@ preprocess_rowmarks(PlannerInfo *root)
continue;
newrc = makeNode(PlanRowMark);
-
newrc->rti = newrc->prti = i;
/* real tables support REFERENCE, anything else needs COPY */
if (rte->rtekind == RTE_RELATION)
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index f48bd31151c..7d4d7b217f7 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -16,7 +16,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.68 2009/10/26 02:26:35 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/optimizer/prep/prepjointree.c,v 1.69 2009/10/28 14:55:38 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1030,6 +1030,12 @@ is_simple_subquery(Query *subquery)
/*
* Can't pull up a subquery involving grouping, aggregation, sorting,
* limiting, or WITH. (XXX WITH could possibly be allowed later)
+ *
+ * We also don't pull up a subquery that has explicit FOR UPDATE/SHARE
+ * clauses, because pullup would cause the locking to occur semantically
+ * higher than it should. Implicit FOR UPDATE/SHARE is okay because
+ * in that case the locking was originally declared in the upper query
+ * anyway.
*/
if (subquery->hasAggs ||
subquery->hasWindowFuncs ||
@@ -1039,6 +1045,7 @@ is_simple_subquery(Query *subquery)
subquery->distinctClause ||
subquery->limitOffset ||
subquery->limitCount ||
+ subquery->hasForUpdate ||
subquery->cteList)
return false;
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 84cbd2142e3..c89c7c82a08 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.394 2009/10/27 17:11:18 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.395 2009/10/28 14:55:43 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -60,8 +60,8 @@ static Query *transformDeclareCursorStmt(ParseState *pstate,
DeclareCursorStmt *stmt);
static Query *transformExplainStmt(ParseState *pstate,
ExplainStmt *stmt);
-static void transformLockingClause(ParseState *pstate,
- Query *qry, LockingClause *lc);
+static void transformLockingClause(ParseState *pstate, Query *qry,
+ LockingClause *lc, bool pushedDown);
static bool check_parameter_resolution_walker(Node *node, ParseState *pstate);
@@ -896,7 +896,8 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
foreach(l, stmt->lockingClause)
{
- transformLockingClause(pstate, qry, (LockingClause *) lfirst(l));
+ transformLockingClause(pstate, qry,
+ (LockingClause *) lfirst(l), false);
}
return qry;
@@ -1348,7 +1349,8 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
foreach(l, lockingClause)
{
- transformLockingClause(pstate, qry, (LockingClause *) lfirst(l));
+ transformLockingClause(pstate, qry,
+ (LockingClause *) lfirst(l), false);
}
return qry;
@@ -2056,7 +2058,8 @@ CheckSelectLocking(Query *qry)
* in rewriteHandler.c, and isLockedRefname() in parse_relation.c.
*/
static void
-transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
+transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc,
+ bool pushedDown)
{
List *lockedRels = lc->lockedRels;
ListCell *l;
@@ -2084,16 +2087,22 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
switch (rte->rtekind)
{
case RTE_RELATION:
- applyLockingClause(qry, i, lc->forUpdate, lc->noWait);
+ applyLockingClause(qry, i,
+ lc->forUpdate, lc->noWait, pushedDown);
rte->requiredPerms |= ACL_SELECT_FOR_UPDATE;
break;
case RTE_SUBQUERY:
-
+ applyLockingClause(qry, i,
+ lc->forUpdate, lc->noWait, pushedDown);
/*
* FOR UPDATE/SHARE of subquery is propagated to all of
- * subquery's rels
+ * subquery's rels, too. We could do this later (based
+ * on the marking of the subquery RTE) but it is convenient
+ * to have local knowledge in each query level about
+ * which rels need to be opened with RowShareLock.
*/
- transformLockingClause(pstate, rte->subquery, allrels);
+ transformLockingClause(pstate, rte->subquery,
+ allrels, true);
break;
default:
/* ignore JOIN, SPECIAL, FUNCTION, VALUES, CTE RTEs */
@@ -2127,16 +2136,17 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
{
case RTE_RELATION:
applyLockingClause(qry, i,
- lc->forUpdate, lc->noWait);
+ lc->forUpdate, lc->noWait,
+ pushedDown);
rte->requiredPerms |= ACL_SELECT_FOR_UPDATE;
break;
case RTE_SUBQUERY:
-
- /*
- * FOR UPDATE/SHARE of subquery is propagated to
- * all of subquery's rels
- */
- transformLockingClause(pstate, rte->subquery, allrels);
+ applyLockingClause(qry, i,
+ lc->forUpdate, lc->noWait,
+ pushedDown);
+ /* see comment above */
+ transformLockingClause(pstate, rte->subquery,
+ allrels, true);
break;
case RTE_JOIN:
ereport(ERROR,
@@ -2190,10 +2200,15 @@ transformLockingClause(ParseState *pstate, Query *qry, LockingClause *lc)
* Record locking info for a single rangetable item
*/
void
-applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
+applyLockingClause(Query *qry, Index rtindex,
+ bool forUpdate, bool noWait, bool pushedDown)
{
RowMarkClause *rc;
+ /* If it's an explicit clause, make sure hasForUpdate gets set */
+ if (!pushedDown)
+ qry->hasForUpdate = true;
+
/* Check for pre-existing entry for same rtindex */
if ((rc = get_parse_rowmark(qry, rtindex)) != NULL)
{
@@ -2207,9 +2222,12 @@ applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
* is a bit more debatable but raising an error doesn't seem helpful.
* (Consider for instance SELECT FOR UPDATE NOWAIT from a view that
* internally contains a plain FOR UPDATE spec.)
+ *
+ * And of course pushedDown becomes false if any clause is explicit.
*/
rc->forUpdate |= forUpdate;
rc->noWait |= noWait;
+ rc->pushedDown &= pushedDown;
return;
}
@@ -2218,6 +2236,7 @@ applyLockingClause(Query *qry, Index rtindex, bool forUpdate, bool noWait)
rc->rti = rtindex;
rc->forUpdate = forUpdate;
rc->noWait = noWait;
+ rc->pushedDown = pushedDown;
qry->rowMarks = lappend(qry->rowMarks, rc);
}
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 0ea80e5e8bc..8f5b3002278 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.189 2009/10/27 17:11:18 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/rewrite/rewriteHandler.c,v 1.190 2009/10/28 14:55:43 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -52,7 +52,7 @@ static Node *get_assignment_input(Node *node);
static void rewriteValuesRTE(RangeTblEntry *rte, Relation target_relation,
List *attrnos);
static void markQueryForLocking(Query *qry, Node *jtnode,
- bool forUpdate, bool noWait);
+ bool forUpdate, bool noWait, bool pushedDown);
static List *matchLocks(CmdType event, RuleLock *rulelocks,
int varno, Query *parsetree);
static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
@@ -1189,23 +1189,13 @@ ApplyRetrieveRule(Query *parsetree,
rte->modifiedCols = NULL;
/*
- * FOR UPDATE/SHARE of view?
+ * 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.
*/
if ((rc = get_parse_rowmark(parsetree, rt_index)) != NULL)
- {
- /*
- * Remove the view from the list of rels that will actually be marked
- * FOR UPDATE/SHARE by the executor. It will still be access-checked
- * for write access, though.
- */
- parsetree->rowMarks = list_delete_ptr(parsetree->rowMarks, rc);
-
- /*
- * Set up the view's referenced tables as if FOR UPDATE/SHARE.
- */
markQueryForLocking(rule_action, (Node *) rule_action->jointree,
- rc->forUpdate, rc->noWait);
- }
+ rc->forUpdate, rc->noWait, true);
return parsetree;
}
@@ -1222,7 +1212,8 @@ ApplyRetrieveRule(Query *parsetree,
* to scan the jointree to determine which rels are used.
*/
static void
-markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait)
+markQueryForLocking(Query *qry, Node *jtnode,
+ bool forUpdate, bool noWait, bool pushedDown)
{
if (jtnode == NULL)
return;
@@ -1233,14 +1224,15 @@ markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait)
if (rte->rtekind == RTE_RELATION)
{
- applyLockingClause(qry, rti, forUpdate, noWait);
+ applyLockingClause(qry, rti, forUpdate, noWait, pushedDown);
rte->requiredPerms |= ACL_SELECT_FOR_UPDATE;
}
else if (rte->rtekind == RTE_SUBQUERY)
{
+ applyLockingClause(qry, rti, forUpdate, noWait, pushedDown);
/* FOR UPDATE/SHARE of subquery is propagated to subquery's rels */
markQueryForLocking(rte->subquery, (Node *) rte->subquery->jointree,
- forUpdate, noWait);
+ forUpdate, noWait, true);
}
/* other RTE types are unaffected by FOR UPDATE */
}
@@ -1250,14 +1242,14 @@ markQueryForLocking(Query *qry, Node *jtnode, bool forUpdate, bool noWait)
ListCell *l;
foreach(l, f->fromlist)
- markQueryForLocking(qry, lfirst(l), forUpdate, noWait);
+ markQueryForLocking(qry, lfirst(l), forUpdate, noWait, pushedDown);
}
else if (IsA(jtnode, JoinExpr))
{
JoinExpr *j = (JoinExpr *) jtnode;
- markQueryForLocking(qry, j->larg, forUpdate, noWait);
- markQueryForLocking(qry, j->rarg, forUpdate, noWait);
+ markQueryForLocking(qry, j->larg, forUpdate, noWait, pushedDown);
+ markQueryForLocking(qry, j->rarg, forUpdate, noWait, pushedDown);
}
else
elog(ERROR, "unrecognized node type: %d",
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8538c74d123..85e52293e76 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -9,7 +9,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.310 2009/10/14 22:14:23 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.311 2009/10/28 14:55:44 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -2549,21 +2549,28 @@ get_select_query_def(Query *query, deparse_context *context,
}
/* Add FOR UPDATE/SHARE clauses if present */
- foreach(l, query->rowMarks)
+ if (query->hasForUpdate)
{
- RowMarkClause *rc = (RowMarkClause *) lfirst(l);
- RangeTblEntry *rte = rt_fetch(rc->rti, query->rtable);
+ foreach(l, query->rowMarks)
+ {
+ RowMarkClause *rc = (RowMarkClause *) lfirst(l);
+ RangeTblEntry *rte = rt_fetch(rc->rti, query->rtable);
- if (rc->forUpdate)
- appendContextKeyword(context, " FOR UPDATE",
- -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
- else
- appendContextKeyword(context, " FOR SHARE",
- -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
- appendStringInfo(buf, " OF %s",
- quote_identifier(rte->eref->aliasname));
- if (rc->noWait)
- appendStringInfo(buf, " NOWAIT");
+ /* don't print implicit clauses */
+ if (rc->pushedDown)
+ continue;
+
+ if (rc->forUpdate)
+ appendContextKeyword(context, " FOR UPDATE",
+ -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
+ else
+ appendContextKeyword(context, " FOR SHARE",
+ -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
+ appendStringInfo(buf, " OF %s",
+ quote_identifier(rte->eref->aliasname));
+ if (rc->noWait)
+ appendStringInfo(buf, " NOWAIT");
+ }
}
context->windowClause = save_windowclause;
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index aa9348a336f..74bf01d29c5 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -37,7 +37,7 @@
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.547 2009/10/26 02:26:41 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.548 2009/10/28 14:55:44 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 200910251
+#define CATALOG_VERSION_NO 200910281
#endif
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 450a89fe85b..2078526092b 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -13,7 +13,7 @@
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.411 2009/10/26 02:26:41 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.412 2009/10/28 14:55:46 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -118,6 +118,7 @@ typedef struct Query
bool hasSubLinks; /* has subquery SubLink */
bool hasDistinctOn; /* distinctClause is from DISTINCT ON */
bool hasRecursive; /* WITH RECURSIVE was specified */
+ bool hasForUpdate; /* FOR UPDATE or FOR SHARE was specified */
List *cteList; /* WITH list (of CommonTableExpr's) */
@@ -803,7 +804,12 @@ typedef struct WindowClause
* parser output representation of FOR UPDATE/SHARE clauses
*
* Query.rowMarks contains a separate RowMarkClause node for each relation
- * identified as a FOR UPDATE/SHARE target.
+ * identified as a FOR UPDATE/SHARE target. If FOR UPDATE/SHARE is applied
+ * to a subquery, we generate RowMarkClauses for all normal and subquery rels
+ * in the subquery, but they are marked pushedDown = true to distinguish them
+ * from clauses that were explicitly written at this query level. Also,
+ * Query.hasForUpdate tells whether there were explicit FOR UPDATE/SHARE
+ * clauses in the current query level.
*/
typedef struct RowMarkClause
{
@@ -811,6 +817,7 @@ typedef struct RowMarkClause
Index rti; /* range table index of target relation */
bool forUpdate; /* true = FOR UPDATE, false = FOR SHARE */
bool noWait; /* NOWAIT option */
+ bool pushedDown; /* pushed down from higher query level? */
} RowMarkClause;
/*
diff --git a/src/include/parser/analyze.h b/src/include/parser/analyze.h
index 59e64c91455..8acd6153f8a 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.42 2009/10/27 17:11:18 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/parser/analyze.h,v 1.43 2009/10/28 14:55:47 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -31,6 +31,6 @@ extern bool analyze_requires_snapshot(Node *parseTree);
extern void CheckSelectLocking(Query *qry);
extern void applyLockingClause(Query *qry, Index rtindex,
- bool forUpdate, bool noWait);
+ bool forUpdate, bool noWait, bool pushedDown);
#endif /* ANALYZE_H */
--
GitLab