Re: SKIP LOCKED DATA (work in progress) - Mailing list pgsql-hackers

From Craig Ringer
Subject Re: SKIP LOCKED DATA (work in progress)
Date
Msg-id 5376ED61.4040506@2ndquadrant.com
Whole thread Raw
In response to Re: SKIP LOCKED DATA (work in progress)  (Thomas Munro <munro@ip9.org>)
Responses Re: SKIP LOCKED DATA (work in progress)
Re: SKIP LOCKED DATA (work in progress)
List pgsql-hackers
On 05/17/2014 05:24 AM, Thomas Munro wrote:

> I noticed that in applyLockingClause, Simon has "rc->waitMode |=
> waitMode". Is that right? The values are 0, 1, and 2, but if you had
> both NOWAIT and SKIP LOCKED somewhere in your query you could up with
> rc->waitMode == 3 unless I'm mistaken.

I do not think that |= is correct there.

It may be that no case can arise where you get the bogus value, but
since in all other places the values are tested for equalty not as bit
fields ( waitMode == NOWAIT not waitMode & NOWAIT ) it doesn't make
sense to |= it there.

? In my patch I have code that
> would give precedence to NOWAIT, though looking at it again it could be
> simpler.

I agree; if NOWAIT is present anywhere it should be preferred to
SKIP_LOCKED, as it's OK to apply NOWAIT where SKIP LOCKED appears, but
possibly semantically incorrect to apply SKIP LOCKED where only NOWAIT
was expected.

> (One reviewer pointed out, that it should really be a single
> unified enum. In fact I have been a bit unsure about what scope such an
> enumeration should have in the application -- could it even be used in
> parser code? I tried to follow existing examples which is why I used
> #define macros in gram.y).

Not sure there.

> From a bikeshed colour point of view:
> * I used SKIP LOCKED DATA  like DB2, and Simon used SKIP LOCKED like
> Oracle, and I guess shorter is sweeter

We have a long tradition of trying to allow noise keywords where it's
harmless.

So the clause should probably be
    SKIP LOCKED [DATA]

in much the same way we have
   BEGIN [ WORK | TRANSACTION ] ...

There won't be any ambiguity there.

> * I used the term wait_policy and an enum, Simon used waitMode and an int

I prefer an enum and intended to change Simon's patch but didn't have
the time.

I have some isolation tester and regression tests that are still to follow.

> * I had noWait and skipLocked travelling separately in some places,
> Simon had a single parameter, which is much better

Yes, I strongly prefer that.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Scaling shared buffer eviction
Next
From: David Rowley
Date:
Subject: Allowing join removals for more join types