Re: Reject WAIT FOR earlier in transaction-snapshot mode

Hi!

On Wed, Sep 16, 2026 at 4:28 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> On Sat, Sep 12, 2026 at 10:48 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> >
> > On Fri, Sep 11, 2026 at 11:58 PM Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> wrote:
> > >
> > > > 1) Could we use 0/0 instead of $lsn3 for these rejection tests? Since
> > > > $lsn3 is deliberately unreachable, the first test can hang if the
> > > > isolation check is missing.
> > >
> > > I think I will keep this as-is. If the isolation-level rejection is
> > > missing, the test is broken.
> > >
> > > > 2) Also, the isolation-error pattern matches the old DETAIL, so
> > > > matching the ERROR: prefix would verify that it is now the primary
> > > > error.
> > >
> > > v4 tightens the new recovery tests so the REPEATABLE READ cases match
> > > the primary ERROR line, rather than the old DETAIL text. I also cleaned
> > > up one test description.
> > >
> > > > The cursor case better additionally check that the misleading
> > > > isolation-level detail is absent.
> > >
> > > I don't think we need that. The cursor case only needs to verify the
> > > new primary error.
> >
> >
> > Thanks. I am OK with your judgement!
>
> Here's a rebase due to a23ab4862cf.

I went through this thread. I don't quite agree with the proposed
patch. I think construction like

BEGIN ISOLATION LEVEL ... ; WAIT FOR LSN ...; do the work

should work at any isolation level. v4 patch makes it work with READ
COMMITTED, but fail with higher levels. I think this is undesired
asymmetry.

I suggest we should better fix the docs. The attached v5 patch does
it. Additionally it makes errdetail() about the isolation level
conditional and shown only on isolation levels higher than READ
COMMITTED.

------
Regards,
Alexander Korotkov
Supabase