On Thu, Sep 17, 2026 at 5:03 AM Xuneng Zhou <[email protected]> wrote: > On Thu, Sep 17, 2026 at 9:43 AM Sami Imseih <[email protected]> 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.
------
Regards,
Alexander Korotkov
Supabase
v6-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch
Description: Binary data
