Robert Haas wrote:
> On Wed, Sep 10, 2014 at 9:47 AM, Alvaro Herrera
> <alvhe...@2ndquadrant.com> wrote:
> > I attach some additional minor suggestions to your patch.
>
> I don't see any attachment.
Uh. Here it is.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 51d1889..2b336b0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -4090,7 +4090,7 @@ get_mxact_status_for_lock(LockTupleMode mode, bool is_update)
* cid: current command ID (used for visibility test, and stored into
* tuple's cmax if lock is successful)
* mode: indicates if shared or exclusive tuple lock is desired
- * wait_policy: whether to block, ereport or skip if lock not available
+ * wait_policy: what to do if tuple lock is not available
* follow_updates: if true, follow the update chain to also lock descendant
* tuples.
*
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index e4065c2..a718c0b 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1928,6 +1928,9 @@ EvalPlanQual(EState *estate, EPQState *epqstate,
*
* Returns a palloc'd copy of the newest tuple version, or NULL if we find
* that there is no newest version (ie, the row was deleted not updated).
+ * We also return NULL if the tuple is locked and the wait policy is to skip
+ * such tuples.
+ *
* If successful, we have locked the newest tuple version, so caller does not
* need to worry about it changing anymore.
*
@@ -1994,7 +1997,7 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
break;
case LockWaitSkip:
if (!ConditionalXactLockTableWait(SnapshotDirty.xmax))
- return NULL; /* skipping instead of waiting */
+ return NULL; /* skip instead of waiting */
break;
case LockWaitError:
if (!ConditionalXactLockTableWait(SnapshotDirty.xmax))
@@ -2076,6 +2079,10 @@ EvalPlanQualFetch(EState *estate, Relation relation, int lockmode,
/* tuple was deleted, so give up */
return NULL;
+ case HeapTupleWouldBlock:
+ elog(WARNING, "this is an odd case");
+ return NULL;
+
default:
ReleaseBuffer(buffer);
elog(ERROR, "unrecognized heap_lock_tuple status: %u",
diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index f9feff4..990240b 100644
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -179,7 +179,10 @@ lnext:
if (copyTuple == NULL)
{
- /* Tuple was deleted or skipped (in SKIP LOCKED), so don't return it */
+ /*
+ * Tuple was deleted; or it's locked and we're under SKIP
+ * LOCKED policy, so don't return it
+ */
goto lnext;
}
/* remember the actually locked tuple's TID */
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 0f6e9b0..1f66928 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -2515,11 +2515,12 @@ applyLockingClause(Query *qry, Index rtindex,
* a shared and exclusive lock at the same time; it'll end up being
* exclusive anyway.)
*
- * We also consider that NOWAIT wins if it is specified multiple ways,
- * otherwise SKIP LOCKED wins. This 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.)
+ * Similarly, if the same RTE is specified with more than one lock wait
+ * policy, consider that NOWAIT wins over SKIP LOCKED, which in turn
+ * wins over waiting for the lock (the default). This 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.
*/
diff --git a/src/include/utils/lockwaitpolicy.h b/src/include/utils/lockwaitpolicy.h
index f8ad07e..7f9a693 100644
--- a/src/include/utils/lockwaitpolicy.h
+++ b/src/include/utils/lockwaitpolicy.h
@@ -1,34 +1,31 @@
/*-------------------------------------------------------------------------
- *
* lockwaitpolicy.h
- * Header file for the enum LockWaitPolicy enum, which is needed in
- * several modules (parser, planner, executor, heap manager).
+ * Header file for LockWaitPolicy enum.
*
* Copyright (c) 2014, PostgreSQL Global Development Group
*
* src/include/utils/lockwaitpolicy.h
- *
*-------------------------------------------------------------------------
*/
#ifndef LOCKWAITPOLICY_H
#define LOCKWAITPOLICY_H
/*
- * Policy for what to do when a row lock cannot be obtained immediately.
- *
- * The enum values defined here control how the parser treats multiple FOR
- * UPDATE/SHARE clauses that affect the same table. If multiple locking
- * clauses are defined then the one with the highest numerical value takes
- * precedence -- see applyLockingClause.
+ * This enum controls how to deal with rows being locked by FOR UPDATE/SHARE
+ * clauses (i.e., NOWAIT and SKIP LOCKED clauses). The ordering here is
+ * important, because the highest numerical value will takes precedence when a
+ * RTE is specified multiple ways. See applyLockingClause.
*/
typedef enum
{
- /* Wait for the lock to become available */
- LockWaitBlock = 1,
- /* SELECT FOR UPDATE SKIP LOCKED, skipping rows that can't be locked */
- LockWaitSkip = 2,
- /* SELECT FOR UPDATE NOWAIT, abandoning the transaction */
- LockWaitError = 3
+ /* Wait for the lock to become available (default behavior) */
+ LockWaitBlock,
+
+ /* Skip rows that can't be locked (SKIP LOCKED) */
+ LockWaitSkip,
+
+ /* Raise an error if a row cannot be locked (NOWAIT) */
+ LockWaitError
} LockWaitPolicy;
#endif /* LOCKWAITPOLICY_H */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
