Reject WAIT FOR earlier in transaction-snapshot mode

Hi Sami,

On Wed, Sep 9, 2026 at 3:55 AM Sami Imseih <samimseih.pg@gmail.com> wrote:

Hi,

The WAIT FOR docs state:

"WAIT FOR must be executed as a top-level command. It cannot be
executed from a function, procedure, or DO block. It also requires
that no active or registered snapshot be held, and therefore cannot be
used in contexts where such a snapshot must remain active, including
transactions running at isolation levels higher than READ COMMITTED."

So this reads as any transaction above READ COMMITTED should reject WAIT FOR.
However, in REPEATABLE READ and SERIALIZABLE, you are allowed to call
WAIT FOR before taking the snapshot.

```
postgres=# BEGIN ISOLATION LEVEL REPEATABLE READ;
BEGIN
postgres=*# WAIT FOR LSN '0/0' WITH (MODE 'primary_flush');
status
---------
success
(1 row)

postgres=*# select 1;
?column?
----------
1
(1 row)

postgres=*# WAIT FOR LSN '0/0' WITH (MODE 'primary_flush');
ERROR: WAIT FOR must be called without an active or registered snapshot
DETAIL: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED.
postgres=!#
```

That does not match the documented behavior, and I do not think there is
a reason to allow WAIT FOR in such a transaction before the snapshot is
taken.

Thanks for the patch! The rationale LGTM.

I've two comments for it:

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.

if (IsolationUsesXactSnapshot())
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAIT FOR cannot be executed in a transaction with
an isolation level higher than READ COMMITTED")));

2) Valid the wait mode in test

+$node_standby->psql(
+ 'postgres',
+ qq[
+BEGIN ISOLATION LEVEL REPEATABLE READ;
+WAIT FOR LSN '${lsn3}' WITH (MODE 'primary_flush');
+],

Primary flush is not a valid mode in standby, so it will still raise
errors without the patch, though the error message is different. After
the patch, the stability of this test relies on the order of errors
between mode and isolation level checking. It might be helpful to
change the mode to default(replay) like WAIT FOR LSN '${lsn3}';

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

On Wed, Sep 9, 2026 at 3:13 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:

Hi Sami,

On Wed, Sep 9, 2026 at 3:55 AM Sami Imseih <samimseih.pg@gmail.com> wrote:

Hi,

The WAIT FOR docs state:

"WAIT FOR must be executed as a top-level command. It cannot be
executed from a function, procedure, or DO block. It also requires
that no active or registered snapshot be held, and therefore cannot be
used in contexts where such a snapshot must remain active, including
transactions running at isolation levels higher than READ COMMITTED."

So this reads as any transaction above READ COMMITTED should reject WAIT FOR.
However, in REPEATABLE READ and SERIALIZABLE, you are allowed to call
WAIT FOR before taking the snapshot.

```
postgres=# BEGIN ISOLATION LEVEL REPEATABLE READ;
BEGIN
postgres=*# WAIT FOR LSN '0/0' WITH (MODE 'primary_flush');
status
---------
success
(1 row)

postgres=*# select 1;
?column?
----------
1
(1 row)

postgres=*# WAIT FOR LSN '0/0' WITH (MODE 'primary_flush');
ERROR: WAIT FOR must be called without an active or registered snapshot
DETAIL: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED.
postgres=!#
```

That does not match the documented behavior, and I do not think there is
a reason to allow WAIT FOR in such a transaction before the snapshot is
taken.

Thanks for the patch! The rationale LGTM.

I've two comments for it:

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.

if (IsolationUsesXactSnapshot())
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAIT FOR cannot be executed in a transaction with
an isolation level higher than READ COMMITTED")));

BEGIN ISOLATION LEVEL READ COMMITTED;
DECLARE c CURSOR FOR SELECT 1;
WAIT FOR LSN '0/0';
-- ERROR: WAIT FOR must be called without an active or registered snapshot

BEGIN ISOLATION LEVEL READ COMMITTED;
SELECT pg_export_snapshot();
WAIT FOR LSN '0/0';
-- ERROR: WAIT FOR must be called without an active or registered snapshot

These are two examples of READ COMMITTED with snapshots remaining unpopped.

2) Valid the wait mode in test

+$node_standby->psql(
+ 'postgres',
+ qq[
+BEGIN ISOLATION LEVEL REPEATABLE READ;
+WAIT FOR LSN '${lsn3}' WITH (MODE 'primary_flush');
+],

Primary flush is not a valid mode in standby, so it will still raise
errors without the patch, though the error message is different. After
the patch, the stability of this test relies on the order of errors
between mode and isolation level checking. It might be helpful to
change the mode to default(replay) like WAIT FOR LSN '${lsn3}';

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

Hi,

Thanks for the review.

BEGIN ISOLATION LEVEL READ COMMITTED;
DECLARE c CURSOR FOR SELECT 1;
WAIT FOR LSN '0/0';
-- ERROR: WAIT FOR must be called without an active or registered snapshot

BEGIN ISOLATION LEVEL READ COMMITTED;
SELECT pg_export_snapshot();
WAIT FOR LSN '0/0';
-- ERROR: WAIT FOR must be called without an active or registered snapshot

This means there is a bug in the ERROR message DETAIL. Thanks for pointing
this out.

```
postgres=# begin;
BEGIN
postgres=*# DECLARE c CURSOR FOR SELECT 1;
DECLARE CURSOR
postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR must be called without an active or registered snapshot
DETAIL: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED.
postgres=!#
```

So I fixed this by first checking the isolation level and using "WAIT
FOR cannot be executed ... READ COMMITTED"
as the errmsg. The existing snapshot check must come after that.

I also added tests to cover the three relevant cases: READ COMMITTED
with a snapshot, and
REPEATABLE READ both with and without a snapshot. The REPEATABLE
READ-with-snapshot
case was already covered, but its expected error message needed to be updated.

The results looks like this:

```
postgres=# BEGIN ISOLATION LEVEL REPEATABLE READ;
BEGIN
postgres=*# select 1;
?column?
----------
1
(1 row)

postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED
postgres=!#
```

```
postgres=# BEGIN ISOLATION LEVEL REPEATABLE READ;
BEGIN
postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED
postgres=!#
```

```
postgres=# BEGIN;
BEGIN
postgres=*# DECLARE c CURSOR FOR SELECT 1;
DECLARE CURSOR
postgres=*#
postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR must be called without an active or registered snapshot
postgres=!#
```

--
Sami

On Wed, Sep 9, 2026 at 9:25 PM Sami Imseih <samimseih.pg@gmail.com> wrote:

Hi,

Thanks for the review.

BEGIN ISOLATION LEVEL READ COMMITTED;
DECLARE c CURSOR FOR SELECT 1;
WAIT FOR LSN '0/0';
-- ERROR: WAIT FOR must be called without an active or registered snapshot

BEGIN ISOLATION LEVEL READ COMMITTED;
SELECT pg_export_snapshot();
WAIT FOR LSN '0/0';
-- ERROR: WAIT FOR must be called without an active or registered snapshot

This means there is a bug in the ERROR message DETAIL. Thanks for pointing
this out.

```
postgres=# begin;
BEGIN
postgres=*# DECLARE c CURSOR FOR SELECT 1;
DECLARE CURSOR
postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR must be called without an active or registered snapshot
DETAIL: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED.
postgres=!#
```

So I fixed this by first checking the isolation level and using "WAIT
FOR cannot be executed ... READ COMMITTED"
as the errmsg. The existing snapshot check must come after that.

I also added tests to cover the three relevant cases: READ COMMITTED
with a snapshot, and
REPEATABLE READ both with and without a snapshot. The REPEATABLE
READ-with-snapshot
case was already covered, but its expected error message needed to be updated.

The results looks like this:

```
postgres=# BEGIN ISOLATION LEVEL REPEATABLE READ;
BEGIN
postgres=*# select 1;
?column?
----------
1
(1 row)

postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED
postgres=!#
```

```
postgres=# BEGIN ISOLATION LEVEL REPEATABLE READ;
BEGIN
postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR cannot be executed within a transaction with an
isolation level higher than READ COMMITTED
postgres=!#
```

```
postgres=# BEGIN;
BEGIN
postgres=*# DECLARE c CURSOR FOR SELECT 1;
DECLARE CURSOR
postgres=*#
postgres=*# WAIT FOR LSN '0/0' WITH (mode 'PRIMARY_FLUSH');
ERROR: WAIT FOR must be called without an active or registered snapshot
postgres=!#

V2 LGTM. Do you prefer to keep the snapshot and isolation-level checks
next to each other?

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

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?

--
Sami

On Wed, Sep 9, 2026 at 10:11 PM Sami Imseih <samimseih.pg@gmail.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.

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.

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.

--
Sami