Re: Reject WAIT FOR earlier in transaction-snapshot mode

On Thu, Sep 17, 2026 at 1:13 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
>
> On Thu, Sep 17, 2026 at 5:03 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> > On Thu, Sep 17, 2026 at 9:43 AM Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> wrote:
> > >
> > > > 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 don't think there is a good use-case here where someone will do:
> > >
> > > BEGIN ISOLATION LEVEL REPEATBLE READ; WAIT FOR LSN ...;
> > > SELECT ...
> >
> > Can you please elaborate on why this is not a good use-case?
> >
> > > but, perhaps it's better to be more flexible here. So, I'm ok with proceeding
> > > with your suggestion.
> >
> > +1. One additional pro for this change is to let users have a sense
> > for the reason the command is being rejected. A blanket rejection of
> > isolation level is somewhat vague since it is an indirect factor. That
> > said, If I was a user who is unable or unwilling to read the source
> > code and dig into mailing list discussion, I would still be confused
> > by the error message and why it is error out. We have detailed commit
> > message/doc/comment for the new execution restriction of not holding
> > heavyweight lock at the onset of waiting. I am wondering whether it
> > could be also helpful to somehow explain the constraints of not
> > holding a snapshot for curious users.
>
> Makes sense. I've added corresponding description to v6.
>
> > > > 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.
> > >
> > > The existing error message is:
> > >
> > > ```
> > > errmsg("WAIT must be called without an active or registered snapshot")
> > > ```
> > >
> > > and v5 uses the same terminology in the documentation:
> > >
> > > ```
> > > <command>DO</command> block. It also requires that no active or
> > > registered snapshot be held.
> > > ```
> > >
> > > The terms "active snapshot" and "registered snapshot" describe internal
> > > snapshot states, and don't seem appropriate for user-facing docs/error
> > > messages.
> > > For example, the COPY documentation instead refers to "snapshots held
> > > by this transaction".
> > > Could we instead use:
> > >
> > > ```
> > > errmsg("WAIT cannot be executed while the current transaction
> > > holds a snapshot")
> > > ```
> > >
> > > and change the documentation to:
> > >
> > > ```
> > > <command>DO</command> block. It also cannot be executed while the
> > > current transaction holds a snapshot. A transaction running at an
> > > isolation level higher than <literal>READ COMMITTED</literal> holds
> > > one from its first query until it ends ....
> >
> > This seems sensible to me.
>
> Done in v6.

Xuneng, Sami, do you think v6 is good enough?

------
Regards,
Alexander Korotkov
Supabase