| 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 |
| 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 |