Re: Reject WAIT FOR earlier in transaction-snapshot mode

On Wed, Sep 9, 2026 at 11:42 PM Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> wrote:
>
> > 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...
>
> I guess it can be moved, but then instead of what we currently see
>
> ```
> postgres=# begin isolation level repeatable read ; WAIT FOR LSN
> '11110/111111111';
> BEGIN
> ERROR: invalid input syntax for type pg_lsn: "11110/111111111"
> postgres=!#
> ```
>
> we will get
> ```
> postgres=# begin isolation level repeatable read ; WAIT FOR LSN
> '11110/111111111';
> BEGIN
> ERROR: WAIT FOR cannot be executed within a transaction with an
> isolation level higher than READ COMMITTED
> postgres=!#
> ```
>
> It is arguable which one is better here, but I am inclined to prefer
> parsing errors first. They tell the user immediately that the supplied
> LSN is invalid. Also, the parsing here is very cheap, so I do not think
> there is much benefit in trying to reject earlier just to save that work.

Yes, this is more about which error fires first. The error-message
style guide only says to give users clear, factual explanations; it
does not state a universal precedence for execution precondition and
argument validation. For empirical reference, I asked Astra to do an
in-tree analysis of the patterns of precedents. Here's what it found:

-- VACUUM
Parse and validate options → check transaction/nested-execution
restrictions → later release the active snapshot

-- REINDEX CONCURRENTLY
Validate option values → check transaction restrictions → resolve the
tablespace name

-- CREATE DATABASE
Check transaction restrictions → validate command options, including
numeric values

-- ALTER SYSTEM
Check transaction restrictions → enter the handler and validate the setting

-- SET TRANSACTION SNAPSHOT
Check transaction state → check isolation level → validate the
snapshot identifier

Both patterns exist in the tree. I am ok with leaving the
isolation-level check and top-level statement check where they are.

> Of course, the existing !isTopLevel restriction behaves differently here,
> since it fires before we parse the WAIT FOR arguments at all. If I were
> inclined to change anything else, I would move both the top-level and
> isolation-level checks to just after parsing but before
>
> This way for top-level we get:
>
> ```
> postgres=# DO $$
> BEGIN
> EXECUTE 'WAIT FOR LSN ''111111111/11''';
> END
> $$;
> ERROR: invalid input syntax for type pg_lsn: "111111111/11"
> CONTEXT: SQL statement "WAIT FOR LSN '111111111/11'"
> PL/pgSQL function inline_code_block line 3 at EXECUTE
> postgres=#
> ```
> instead of
>
> ```
> postgres=# DO $$
> BEGIN
> EXECUTE 'WAIT FOR LSN ''0/0''';
> END
> $$;
> ERROR: WAIT FOR can only be executed as a top-level statement
> DETAIL: WAIT FOR cannot be used within a function, procedure, or DO block.
> CONTEXT: SQL statement "WAIT FOR LSN '0/0'"
> PL/pgSQL function inline_code_block line 3 at EXECUTE
> postgres=#
> ```
>
> but not sure that's worth it either.

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