Re: FIX: BUG #19687: ALTER SEQUENCE missing lock

Hi,

On Sun, 27 Sept 2026 at 12:06, Alexandre Felipe <
o(dot)alexandre(dot)felipe(at)gmail(dot)com> wrote:
>
> On Tue, Sep 15, 2026 at 9:00 AM Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
wrote:
>>
>> I posted about the same bug on pgsql-bugs yesterday [2] (quoted below),
>> so maybe it is better to keep this in one thread.
>
>
> Hi Ayush, do you want to take care of this, since you already did some
good work on it?
>
> Thank you for your review Andrew
>
> On Sat, Sep 26, 2026 at 11:46 PM Andrew Krylosov <
krylosov(dot)andrew(at)gmail(dot)com> wrote:
>>
>> Hi,
>>
>> I applied v1 on top of 3c5d9d914f and built it with cassert. regress
>> and isolation pass, and Alexander's script no longer fails for me
>> (200 iterations, with and without the forced parallel plan; on HEAD
>> it fails within 50).
>>
>> However, the lock upgrade in v1 is exactly what Ayush described
>> upthread, not a lock inside RelationSetNewRelfilenumber():
>>
>> + LockRelationOid(relid, AccessExclusiveLock);
>>
>> At this point we already hold ShareRowExclusiveLock, so a transaction
>> that has read the sequence and then calls nextval() deadlocks with us:
>>
>> s2: BEGIN; SELECT last_value FROM seq1;
>> s1: BEGIN; ALTER SEQUENCE seq1 AS int; -- waits
>> s2: SELECT nextval('seq1');
>> ERROR: deadlock detected
>>
>> The same deadlock occurs if s2 runs ALTER SEQUENCE instead of
>> nextval(). On HEAD, s2 waits for s1 in both cases.
>>
>> Taking AccessExclusiveLock at the initial RangeVarGetRelidExtended()
>> call avoids these deadlocks: s1 waits for s2, and s2 can finish.
>> This also fixes the reported failures. ResetSequence() and
>> SequenceChangePersistence() already use this lock mode.
>>
>> This change was also discussed in the bug thread:
>> https://postgr.es/m/19687-dd094472b0c48afb@postgresql.org
>>
>> The downside is that ALTER SEQUENCE ... OWNED BY alone would also
>> block readers, even though it doesn't rewrite the sequence. We could
>> choose the lock mode from the option list up front to preserve the
>> current behavior for that case, but I'm not sure the extra complexity
>> is worth it.
>>
>> AccessExclusiveLock also covers hot standby, since it is WAL-logged.
>> On HEAD, a standby query that reads the sequence across the replay of
>> the ALTER's commit fails with
>>
>> ERROR: could not open file "base/5/16384": No such file or directory
>>
>> while with the stronger lock it gets the usual recovery conflict.
>>
>> Also I think the patch needs a test. An isolation permutation in
>> sequence-ddl.spec with a cursor open on the sequence works
>> deterministically: on HEAD the ALTER doesn't wait and the following
>> FETCH silently returns zero rows, with the fix the ALTER waits.
>> A SELECT-then-nextval() permutation would catch the deadlock above.
>>
>> The new comment isn't quite accurate: RelationSetNewRelfilenumber()
>> doesn't destroy the old file, it is dropped at commit while other
>> backends may still be reading it.

I'll prepare a patch once we get an initial review from SMEs.

Let's please use that bug thread [1] for discussion and not continue here.

Regards,
Ayush

[1]
https://www.postgresql.org/message-id/CAJTYsWVeKcAsKCS7YiQJT879SkooJrq1bEXLfvEH%2BKasjNSi1A%40mail.gmail.com