Re: Reject WAIT FOR earlier in transaction-snapshot mode

From: Xuneng Zhou <xunengzhou(at)gmail(dot)com>
To: Sami Imseih <samimseih(dot)pg(at)gmail(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Reject WAIT FOR earlier in transaction-snapshot mode
Date: 2026-09-09 14:50:42
Message-ID: CABPTF7U_dfVfk9_pXqHfhPap9-YVP8fk-ms48Lc1GF2BKsSw1w@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 9, 2026 at 10:11 PM Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> wrote:
>
> > V2 LGTM.
>
> Thanks!
>
> > Do you prefer to keep the snapshot and isolation-level checks
> > next to each other?
>
> Can you elaborate on this point? Do you mean combine them in
> a single block?

1) Separate the isolation check from the snapshot check
I am wondering whether it could be helpful to move the isolation level
check to the place after the top level statement checking. It has two
pros -- one is to distinguish isolation levels higher than READ
COMMITTED from READ COMMITTED with snapshots remaining unpopped;
another is to reject early in order to save some parsing effort for
unsupported use.

I might not express this clearly in the first email I replied
upthread. Currently, we pop and invalidate the snapshot after the
grammar parsing and argument validation. That placement is sensible,
but the inverse order of them seems not, because it is not meaningful
to clean-up snapshots for already invalid inputs and it also invites
confusion for error reporting if snapshot and input handling both went
wrong. However, moving the new separate isolation-level check to the
very beginning of the function immediately after the existing
top-level statement check somehow make sense to me:

/*
* WAIT FOR must not be run as a non-top-level statement (e.g., inside a
* function, procedure, or DO block). Forbid this case upfront.
*/
if (!isTopLevel)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("%s can only be executed as a top-level statement",
"WAIT FOR"),
errdetail("WAIT FOR cannot be used within a function, procedure, or DO
block.")));

The rationale for this move is to reject unsupported use early so as
to save some parsing/invalidation effort. The downside of this is that
it scatters the logic of snapshot management, which is the core of all
the existing restrictions like running as top-level statements under
selected isolation levels. That said, we already make the rejection of
non-top-level statements early in the function, which means they are
not clustered even for now...

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2026-09-09 14:54:18 Re: remove_useless_joins vs. bug #19560
Previous Message Андрей Казаринов 2026-09-09 14:41:06 Re: [PATCH] Allow subquery pull-up past inlineable CTEs