References: <17141-913d78b9675aac8e@postgresql.org>
 <202108112007.evylbmzueapg@alvherre.pgsql>
User-agent: mu4e 1.4.15; emacs 27.1
From: David Christensen <david.christensen@crunchydata.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: emil@iggland.com, pgsql-bugs@lists.postgresql.org, Surafel Temesgen
 <surafel3000@gmail.com>
Subject: Re: BUG #17141: SELECT LIMIT WITH TIES FOR UPDATE SKIP LOCKED
 returns wrong number of rows
In-reply-to: <202108112007.evylbmzueapg@alvherre.pgsql>
Date: Wed, 11 Aug 2021 16:39:00 -0500
Message-ID: <lzk0krveyz.fsf@crunchydata.com>
MIME-Version: 1.0
Content-Type: text/plain
Archived-At: 
 <https://www.postgresql.org/message-id/lzk0krveyz.fsf%40crunchydata.com>
Precedence: bulk


Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

> On 2021-Aug-11, PG Bug reporting form wrote:
>
>> BEGIN; 
>> SELECT * FROM queue
>> ORDER BY task DESC
>> FETCH FIRST 1 ROWS WITH TIES
>> FOR UPDATE SKIP LOCKED;
>> /* Some work to be done here */
>> COMMIT;
>> 
>> select version();
>> PostgreSQL 13.3, compiled by Visual C++ build 1914, 64-bit
>> 
>> Expected result Worker 1: (580), (580), Actual result Worker 1: (580), (580)
>> Expected result Worker 2: (480), (480), Actual result Worker 2: (480)
>
> Ouch, we already saw this actually:
> https://postgr.es/m/16676-fd62c3c835880da6@postgresql.org
> The problem is that the first worker locks the first (480) row (even
> though it does not return it), so the second worker skips it due to SKIP
> LOCKED.
>
> I have this on my list of things to look at, but it's not at the top
> yet sadly ...

Yeah, I'd looked at this when I found it, and short of detecting the situation "WITH TIES FOR UPDATE
SKIP LOCKED" and erroring out, it seems like it would require adding in infrastructure that we don't
support (AFAIK) with unlocking an already locked row inside a transaction or reworking the order of
LockRows and Limit such that Limit comes first (and itself handles the WITH TIES) before handing to
LockRows.  Either way (other than the error), it seems to be a fairly invasive change.

If someone has another idea on how to handle this, I could take a stab at things.  Detecting the
situation and erroring seems like the easiest way to handle so you're at least not getting back bad
results, though I agree that the functionality would be useful if we *could* support it somehow.

David
--