Re: SKIP LOCKED DATA (work in progress)

That seems much cleaner

A few more comments:

You seem to have lost the comment which indicates that the values of the enum are important due to the code in applyLockingClause(), but I see now instead that you've added some assert checks in applyLockingClause(), likely these should use Assert() rather than StaticAssertExpr().

I've also been looking at the isolation tests and I see that you've added a series of tests for NOWAIT. I was wondering why you did that as that's really existing code, probably if you thought the tests were a bit thin around NOWAIT then maybe that should be a separate patch?

In ExecLockRows(), is there a need to define the wait_policy variable now? It's just used once so you could probably just pass erm->waitPolicy directly to heap_lock_tuple().

I'm a bit confused at some of the code in heap_lock_tuple(). If I'm not wrong then after the line that does have_tuple_lock = true; it's never possible for have_tuple_lock to be false, but I see you've added checks to ensure we only unlock if have_tuple_lock is true. I'm thinking you probably did this because in the goto failed situation the check is done, but I was thinking that was perhaps put there in case a goto jump was added before have_tuple_lock is set to true. I'm wondering if it would be ok just to replace the test with an Assert() instead, or maybe just no check. 

Also, I'm just looking at some of the changes that you've done to function signatures... I see quite a few of them are now beyond 80 chars wide (see http://www.postgresql.org/docs/devel/static/source-format.html). 

Regards

David Rowley