From 46e3a16b050a23b924e5d8a75c8bb7068c26aa96 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 28 Oct 2009 14:55:47 +0000 Subject: 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. --- src/backend/utils/adt/ruleutils.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) (limited to 'src/backend/utils/adt/ruleutils.c') 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; -- cgit v1.2.3