Re: Reject WAIT FOR earlier in transaction-snapshot mode

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: Xuneng Zhou <xunengzhou(at)gmail(dot)com>
Cc: Sami Imseih <samimseih(dot)pg(at)gmail(dot)com>, Álvaro Herrera <alvherre(at)kurilemu(dot)de>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Reject WAIT FOR earlier in transaction-snapshot mode
Date: 2026-09-17 10:13:26
Message-ID: CAPpHfdvRoeM8=FN7GvUay820TG9GSEwP1NorSSLVFJik0HrOqw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

------
Regards,
Alexander Korotkov
Supabase

Attachment Content-Type Size
v6-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch application/octet-stream 7.2 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2026-09-17 10:33:17 Re: Distinguish publication exclusions in object addresses
Previous Message Florin Irion 2026-09-17 10:10:13 Re: Proposal: Supporting URI SAN in Certificate Authentication