| From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
|---|---|
| To: | Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> |
| Cc: | Alexander Korotkov <aekorotkov(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 02:03:12 |
| Message-ID: | CABPTF7X+iVLUrYZcFwHmuV56_er0H+9mWvAkR8cPM4dVzvQAXA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
> > 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.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shihao zhong | 2026-09-17 02:03:49 | Re: REPACK enhancements |
| Previous Message | ZizhuanLiu X-MAN | 2026-09-17 02:01:24 | Re: Optimize MCV stats for sortable types and utilize sorted-order properties |