pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedReject WAIT FOR earlier in transaction-snapshot mode
36+ messages / 5 participants
[nested] [flat]
* Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-08 19:55 Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-08 19:55 UTC (permalink / raw)
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
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.
--
Sami Imseih
Amazon Web Services (AWS)
Attachments:
[application/octet-stream] v1-0001-Reject-WAIT-FOR-earlier-in-transaction-snapshot-m.patch (3.1K, ../../CAN12+YJddgAZzA36JZFX+YrnNR7Uubp9qWFz6ASs3v0kL30AMA@mail.gmail.com/2-v1-0001-Reject-WAIT-FOR-earlier-in-transaction-snapshot-m.patch)
download | inline diff:
From e88a42f4832a015ab185bc2edd48f986f6fbdc2e Mon Sep 17 00:00:00 2001
From: Sami Imseih <samimseih.pg@gmail.com>
Date: Tue, 8 Sep 2026 19:34:13 +0000
Subject: [PATCH v1 1/1] Reject WAIT FOR earlier in transaction-snapshot mode
The documentation says that WAIT FOR cannot be used in transactions above
READ COMMITTED, but the implementation only rejected it after a transaction
snapshot had already been taken.
Make WAIT FOR fail earlier in transaction-snapshot mode so the behavior
matches the documented restriction and the intended invariant.
Add a recovery test that covers the REPEATABLE READ case before any
statement has forced the transaction snapshot.
---
src/backend/commands/wait.c | 9 +++++++--
src/test/recovery/t/049_wait_for_lsn.pl | 14 +++++++++++++-
2 files changed, 20 insertions(+), 3 deletions(-)
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 9ba4c75021e..7d8f41e2736 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -15,6 +15,7 @@
#include <math.h>
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -167,8 +168,12 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
*/
InvalidateCatalogSnapshot();
- /* Give up if there is still an active or registered snapshot. */
- if (HaveRegisteredOrActiveSnapshot())
+ /*
+ * Give up if there is still an active or registered snapshot. Also reject
+ * transactions that use a transaction snapshot, even if that snapshot has
+ * not been taken yet.
+ */
+ if (HaveRegisteredOrActiveSnapshot() || IsolationUsesXactSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAIT FOR must be called without an active or registered snapshot"),
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index cb7d4d461de..5faabc8b0a3 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -264,7 +264,7 @@ unlike(
# 5. Check mode validation: standby modes error on primary, primary mode errors
# on standby, and primary_flush works on primary. Also check that WAIT FOR
# triggers an error if called within a function, procedure, anonymous DO block,
-# or inside a transaction with an isolation level higher than READ COMMITTED.
+# or inside a transaction that uses a transaction snapshot.
# Test standby_flush on primary - should error
$node_primary->psql(
@@ -291,6 +291,18 @@ ok( $stderr =~
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+$node_standby->psql(
+ 'postgres',
+ qq[
+BEGIN ISOLATION LEVEL REPEATABLE READ;
+WAIT FOR LSN '${lsn3}' WITH (MODE 'primary_flush');
+],
+ stderr => \$stderr);
+ok( $stderr =~
+ /WAIT FOR must be called without an active or registered snapshot/,
+ "get an error before taking a repeatable read transaction snapshot"
+);
+
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
# should error
$node_primary->safe_psql(
--
2.50.1
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 07:13 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-09 07:13 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 07:30 Xuneng Zhou <xunengzhou@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-09 07:30 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 13:25 Sami Imseih <samimseih.pg@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 2 replies; 36+ messages in thread
From: Sami Imseih @ 2026-09-09 13:25 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
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
Attachments:
[application/octet-stream] v2-0001-Fix-WAIT-FOR-rejection-errors.patch (4.5K, ../../CAN12+YL=5a24oTYSj_Qs0Y80spS0MDwX5egcriXBqdHgpGiX4Q@mail.gmail.com/2-v2-0001-Fix-WAIT-FOR-rejection-errors.patch)
download | inline diff:
From 623417efd4cdbde8cb8f52c294fd24ba639babd1 Mon Sep 17 00:00:00 2001
From: Sami Imseih <samimseih.pg@gmail.com>
Date: Wed, 9 Sep 2026 12:29:22 +0000
Subject: [PATCH v2 1/1] Fix WAIT FOR rejection errors
WAIT FOR is documented as unsupported in transactions above READ
COMMITTED, but the implementation only rejected that case after a
transaction snapshot had already been taken. As a result, higher
isolation levels could reach the generic snapshot-check error instead
of failing with an error indicating that the isolation level is not
supported.
The same "active or registered snapshot" error also included an
isolation-level explanation as DETAIL, which is wrong for READ
COMMITTED cases such as a held cursor.
Reject transaction-snapshot mode explicitly before checking for
registered or active snapshots, so REPEATABLE READ and SERIALIZABLE
fail with the right message and snapshot-holding READ COMMITTED cases
report only the snapshot-specific error.
Add recovery coverage for both cases.
---
src/backend/commands/wait.c | 11 +++++++++--
src/test/recovery/t/049_wait_for_lsn.pl | 22 ++++++++++++++++++++--
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 9ba4c75021e..604e45e36fc 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -18,6 +18,7 @@
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
+#include "access/xact.h"
#include "catalog/pg_type_d.h"
#include "commands/defrem.h"
#include "commands/wait.h"
@@ -167,12 +168,18 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
*/
InvalidateCatalogSnapshot();
+ /* Give up if we are in a transaction that uses a transaction snapshot. */
+ if (IsolationUsesXactSnapshot())
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED"));
+
+
/* Give up if there is still an active or registered snapshot. */
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT FOR must be called without an active or registered snapshot"),
- errdetail("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT FOR must be called without an active or registered snapshot"));
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index cb7d4d461de..4b6e5194579 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -264,7 +264,7 @@ unlike(
# 5. Check mode validation: standby modes error on primary, primary mode errors
# on standby, and primary_flush works on primary. Also check that WAIT FOR
# triggers an error if called within a function, procedure, anonymous DO block,
-# or inside a transaction with an isolation level higher than READ COMMITTED.
+# or inside a transaction that uses a transaction snapshot.
# Test standby_flush on primary - should error
$node_primary->psql(
@@ -282,13 +282,31 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~
+ /WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get an error when running WAIT FOR in a transaction with an isolation level higher than READ COMMITTED"
+);
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
+ok( $stderr =~
+ /WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get the isolation-level error even after a transaction snapshot has been taken"
+);
+
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL READ COMMITTED; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
ok( $stderr =~
/WAIT FOR must be called without an active or registered snapshot/,
- "get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
+ "get an error when running WAIT FOR with an active or registered snapshot"
);
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
--
2.50.1
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 14:00 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
1 sibling, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-09 14:00 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 14:11 Sami Imseih <samimseih.pg@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-09 14:11 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
> 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
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 14:50 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-09 14:50 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
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.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 15:24 Álvaro Herrera <alvherre@kurilemu.de>
parent: Sami Imseih <samimseih.pg@gmail.com>
1 sibling, 1 reply; 36+ messages in thread
From: Álvaro Herrera @ 2026-09-09 15:24 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
Hi,
Not critically important but note that the PopActiveSnapshot and
InvalidateCatalogSnapshot call and the comment about "giving up if there
is still an active or registered snapshot" have a close relationship; I
think it would make sense to keep them together. I would add your new
test-and-ereport just before the PopActiveSnapshot() call, to avoid
breaking that.
Cheers
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Siempre hay que alimentar a los dioses, aunque la tierra esté seca" (Orual)
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-09 15:41 Sami Imseih <samimseih.pg@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-09 15:41 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
> 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
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-10 01:59 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 0 replies; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-10 01:59 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Wed, Sep 9, 2026 at 11:42 PM Sami Imseih <samimseih.pg@gmail.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.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-10 11:02 Xuneng Zhou <xunengzhou@gmail.com>
parent: Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-10 11:02 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
Hi Álvaro,
On Thu, Sep 10, 2026 at 6:21 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>
> Hi,
>
> Not critically important but note that the PopActiveSnapshot and
> InvalidateCatalogSnapshot call and the comment about "giving up if there
> is still an active or registered snapshot" have a close relationship; I
> think it would make sense to keep them together. I would add your new
> test-and-ereport just before the PopActiveSnapshot() call, to avoid
> breaking that.
Good catch. +1 for doing that.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-10 17:40 Sami Imseih <samimseih.pg@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-10 17:40 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
> > Not critically important but note that the PopActiveSnapshot and
> > InvalidateCatalogSnapshot call and the comment about "giving up if there
> > is still an active or registered snapshot" have a close relationship; I
> > think it would make sense to keep them together. I would add your new
> > test-and-ereport just before the PopActiveSnapshot() call, to avoid
> > breaking that.
>
> Good catch. +1 for doing that.
Sure, that works. See the attached.
I think as this is an error reporting bug, maybe we should backpatch to 19?
Sami Imseih
Amazon Web Services (AWS)
Attachments:
[application/octet-stream] v3-0001-Fix-WAIT-FOR-rejection-errors.patch (4.7K, ../../CAN12+Y+j+aKihcuE7ruGn-rYp_=AAxifLJz+BDsu+Nb2EJb+Uw@mail.gmail.com/2-v3-0001-Fix-WAIT-FOR-rejection-errors.patch)
download | inline diff:
From 5a05e1fd940b1bad240672b48c45b3c91e008006 Mon Sep 17 00:00:00 2001
From: Sami Imseih <samimseih.pg@gmail.com>
Date: Wed, 9 Sep 2026 12:29:22 +0000
Subject: [PATCH v3 1/1] Fix WAIT FOR rejection errors
WAIT FOR is documented as unsupported in transactions above READ
COMMITTED, but the implementation only rejected that case after a
transaction snapshot had already been taken. As a result, higher
isolation levels could reach the generic snapshot-check error instead
of failing with an error indicating that the isolation level is not
supported.
The same "active or registered snapshot" error also included an
isolation-level explanation as DETAIL, which is wrong for READ
COMMITTED cases such as a held cursor.
Reject transaction-snapshot mode explicitly before checking for
registered or active snapshots, so REPEATABLE READ and SERIALIZABLE
fail with the right message and snapshot-holding READ COMMITTED cases
report only the snapshot-specific error.
Add recovery coverage for both cases.
---
src/backend/commands/wait.c | 13 +++++++++++--
src/test/recovery/t/049_wait_for_lsn.pl | 22 ++++++++++++++++++++--
2 files changed, 31 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 9ba4c75021e..7240ce08bb3 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -18,6 +18,7 @@
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
+#include "access/xact.h"
#include "catalog/pg_type_d.h"
#include "commands/defrem.h"
#include "commands/wait.h"
@@ -147,6 +148,15 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
}
}
+ /*
+ * WAIT FOR must not be run in a transaction that uses a transaction
+ * snapshot.
+ */
+ if (IsolationUsesXactSnapshot())
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED"));
+
/*
* We are going to wait for the LSN. We should first care that we don't
* hold a snapshot and correspondingly our MyProc->xmin is invalid.
@@ -171,8 +181,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT FOR must be called without an active or registered snapshot"),
- errdetail("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT FOR must be called without an active or registered snapshot"));
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index cb7d4d461de..4b6e5194579 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -264,7 +264,7 @@ unlike(
# 5. Check mode validation: standby modes error on primary, primary mode errors
# on standby, and primary_flush works on primary. Also check that WAIT FOR
# triggers an error if called within a function, procedure, anonymous DO block,
-# or inside a transaction with an isolation level higher than READ COMMITTED.
+# or inside a transaction that uses a transaction snapshot.
# Test standby_flush on primary - should error
$node_primary->psql(
@@ -282,13 +282,31 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~
+ /WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get an error when running WAIT FOR in a transaction with an isolation level higher than READ COMMITTED"
+);
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
+ok( $stderr =~
+ /WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get the isolation-level error even after a transaction snapshot has been taken"
+);
+
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL READ COMMITTED; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
ok( $stderr =~
/WAIT FOR must be called without an active or registered snapshot/,
- "get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
+ "get an error when running WAIT FOR with an active or registered snapshot"
);
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
--
2.50.1
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-11 02:24 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-11 02:24 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 11, 2026 at 1:40 AM Sami Imseih <samimseih.pg@gmail.com> wrote:
>
> > > Not critically important but note that the PopActiveSnapshot and
> > > InvalidateCatalogSnapshot call and the comment about "giving up if there
> > > is still an active or registered snapshot" have a close relationship; I
> > > think it would make sense to keep them together. I would add your new
> > > test-and-ereport just before the PopActiveSnapshot() call, to avoid
> > > breaking that.
> >
> > Good catch. +1 for doing that.
>
> Sure, that works. See the attached.
Thanks.
> I think as this is an error reporting bug, maybe we should backpatch to 19?
+1 for backpatch. The patch suffices for that purpose, since there's
no divergence between v19 and HEAD for this feature currently. For
cautiousness, I also asked for an AI review for this patch. Here's
what it suggested:
1) Could we use 0/0 instead of $lsn3 for these rejection tests? Since
$lsn3 is deliberately unreachable, the first test can hang if the
isolation check is missing.
2) Also, the isolation-error pattern matches the old DETAIL, so
matching the ERROR: prefix would verify that it is now the primary
error. The cursor case better additionally check that the misleading
isolation-level detail is absent.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-11 15:58 Sami Imseih <samimseih.pg@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-11 15:58 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> 1) Could we use 0/0 instead of $lsn3 for these rejection tests? Since
> $lsn3 is deliberately unreachable, the first test can hang if the
> isolation check is missing.
I think I will keep this as-is. If the isolation-level rejection is
missing, the test is broken.
> 2) Also, the isolation-error pattern matches the old DETAIL, so
> matching the ERROR: prefix would verify that it is now the primary
> error.
v4 tightens the new recovery tests so the REPEATABLE READ cases match
the primary ERROR line, rather than the old DETAIL text. I also cleaned
up one test description.
> The cursor case better additionally check that the misleading
> isolation-level detail is absent.
I don't think we need that. The cursor case only needs to verify the
new primary error.
--
Sami Imseih
Amazon Web Services (AWS)
Attachments:
[application/octet-stream] v4-0001-Fix-WAIT-FOR-rejection-errors.patch (5.0K, ../../CAN12+YKU=7_qeOV1Sa53Ux6XC9Vp-oLRJrfeV=ie=oM3-8SzKA@mail.gmail.com/2-v4-0001-Fix-WAIT-FOR-rejection-errors.patch)
download | inline diff:
From cc318cc38614481cf5e95b7ddb30643e356b3f23 Mon Sep 17 00:00:00 2001
From: Sami Imseih <samimseih.pg@gmail.com>
Date: Wed, 9 Sep 2026 12:29:22 +0000
Subject: [PATCH v4 1/1] Fix WAIT FOR rejection errors
WAIT FOR is documented as unsupported in transactions above READ
COMMITTED, but the implementation only rejected that case after a
transaction snapshot had already been taken. As a result, higher
isolation levels could reach the generic snapshot-check error instead
of failing with an error indicating that the isolation level is not
supported.
The same "active or registered snapshot" error also included an
isolation-level explanation as DETAIL, which is wrong for READ
COMMITTED cases such as a held cursor.
Reject transaction-snapshot mode explicitly before checking for
registered or active snapshots, so REPEATABLE READ and SERIALIZABLE
fail with the right message and snapshot-holding READ COMMITTED cases
report the snapshot-specific error.
Add recovery coverage for both cases.
---
src/backend/commands/wait.c | 13 ++++++++++--
src/test/recovery/t/049_wait_for_lsn.pl | 28 ++++++++++++++++++++-----
2 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 9ba4c75021e..7240ce08bb3 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -18,6 +18,7 @@
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
+#include "access/xact.h"
#include "catalog/pg_type_d.h"
#include "commands/defrem.h"
#include "commands/wait.h"
@@ -147,6 +148,15 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
}
}
+ /*
+ * WAIT FOR must not be run in a transaction that uses a transaction
+ * snapshot.
+ */
+ if (IsolationUsesXactSnapshot())
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED"));
+
/*
* We are going to wait for the LSN. We should first care that we don't
* hold a snapshot and correspondingly our MyProc->xmin is invalid.
@@ -171,8 +181,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT FOR must be called without an active or registered snapshot"),
- errdetail("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT FOR must be called without an active or registered snapshot"));
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index cb7d4d461de..934b2b121e6 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -262,9 +262,9 @@ unlike(
"WAIT FOR LSN after savepoint rollback did not disconnect");
# 5. Check mode validation: standby modes error on primary, primary mode errors
-# on standby, and primary_flush works on primary. Also check that WAIT FOR
-# triggers an error if called within a function, procedure, anonymous DO block,
-# or inside a transaction with an isolation level higher than READ COMMITTED.
+# on standby. Also check that WAIT FOR triggers an error if called within a
+# function, procedure, anonymous DO block, in a transaction with an isolation
+# level higher than READ COMMITTED, or with an active or registered snapshot.
# Test standby_flush on primary - should error
$node_primary->psql(
@@ -282,13 +282,31 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok($stderr =~
+ /ERROR:\s+WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get an error when running WAIT FOR in a transaction with an isolation level higher than READ COMMITTED"
+);
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
-ok( $stderr =~
+ok($stderr =~
+ /ERROR:\s+WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get an isolation-level error when running WAIT FOR after taking a transaction snapshot"
+);
+
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL READ COMMITTED; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok($stderr =~
/WAIT FOR must be called without an active or registered snapshot/,
- "get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
+ "get an error when running WAIT FOR with an active or registered snapshot"
);
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
--
2.50.1
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-12 02:48 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-12 02:48 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 11, 2026 at 11:58 PM Sami Imseih <samimseih.pg@gmail.com> wrote:
>
> > 1) Could we use 0/0 instead of $lsn3 for these rejection tests? Since
> > $lsn3 is deliberately unreachable, the first test can hang if the
> > isolation check is missing.
>
> I think I will keep this as-is. If the isolation-level rejection is
> missing, the test is broken.
>
> > 2) Also, the isolation-error pattern matches the old DETAIL, so
> > matching the ERROR: prefix would verify that it is now the primary
> > error.
>
> v4 tightens the new recovery tests so the REPEATABLE READ cases match
> the primary ERROR line, rather than the old DETAIL text. I also cleaned
> up one test description.
>
> > The cursor case better additionally check that the misleading
> > isolation-level detail is absent.
>
> I don't think we need that. The cursor case only needs to verify the
> new primary error.
Thanks. I am OK with your judgement!
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-16 01:28 Xuneng Zhou <xunengzhou@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
0 siblings, 2 replies; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-16 01:28 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Sat, Sep 12, 2026 at 10:48 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> On Fri, Sep 11, 2026 at 11:58 PM Sami Imseih <samimseih.pg@gmail.com> wrote:
> >
> > > 1) Could we use 0/0 instead of $lsn3 for these rejection tests? Since
> > > $lsn3 is deliberately unreachable, the first test can hang if the
> > > isolation check is missing.
> >
> > I think I will keep this as-is. If the isolation-level rejection is
> > missing, the test is broken.
> >
> > > 2) Also, the isolation-error pattern matches the old DETAIL, so
> > > matching the ERROR: prefix would verify that it is now the primary
> > > error.
> >
> > v4 tightens the new recovery tests so the REPEATABLE READ cases match
> > the primary ERROR line, rather than the old DETAIL text. I also cleaned
> > up one test description.
> >
> > > The cursor case better additionally check that the misleading
> > > isolation-level detail is absent.
> >
> > I don't think we need that. The cursor case only needs to verify the
> > new primary error.
>
>
> Thanks. I am OK with your judgement!
Here's a rebase due to a23ab4862cf.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
Attachments:
[application/octet-stream] v5-0001-Fix-WAIT-FOR-rejection-errors.patch (5.0K, ../../CABPTF7XK82dGsgry_8HSkSD=zapGH6LHpoLxNUMV027yNptsfA@mail.gmail.com/2-v5-0001-Fix-WAIT-FOR-rejection-errors.patch)
download | inline diff:
From a1bb1d62db7136f40e5cbb17ef3bdaa9e29a8639 Mon Sep 17 00:00:00 2001
From: Sami Imseih <samimseih.pg@gmail.com>
Date: Wed, 9 Sep 2026 12:29:22 +0000
Subject: [PATCH v5] Fix WAIT FOR rejection errors
WAIT FOR is documented as unsupported in transactions above READ
COMMITTED, but the implementation only rejected that case after a
transaction snapshot had already been taken. As a result, higher
isolation levels could reach the generic snapshot-check error instead
of failing with an error indicating that the isolation level is not
supported.
The same "active or registered snapshot" error also included an
isolation-level explanation as DETAIL, which is wrong for READ
COMMITTED cases such as a held cursor.
Reject transaction-snapshot mode explicitly before checking for
registered or active snapshots, so REPEATABLE READ and SERIALIZABLE
fail with the right message and snapshot-holding READ COMMITTED cases
report the snapshot-specific error.
Add recovery coverage for both cases.
---
src/backend/commands/wait.c | 13 +++++++++--
src/test/recovery/t/049_wait_for_lsn.pl | 29 ++++++++++++++++++++-----
2 files changed, 35 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 2223e91a3e3..87c4651a1a0 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -13,6 +13,7 @@
*/
#include "postgres.h"
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -131,6 +132,15 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
}
}
+ /*
+ * WAIT FOR must not be run in a transaction that uses a transaction
+ * snapshot.
+ */
+ if (IsolationUsesXactSnapshot())
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED"));
+
/*
* We are going to wait for the LSN. We should first care that we don't
* hold a snapshot and correspondingly our MyProc->xmin is invalid.
@@ -155,8 +165,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT must be called without an active or registered snapshot"),
- errdetail("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT must be called without an active or registered snapshot"));
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 3bc936e0f91..20389a576d6 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -262,9 +262,9 @@ unlike(
"WAIT FOR LSN after savepoint rollback did not disconnect");
# 5. Check mode validation: standby modes error on primary, primary mode errors
-# on standby, and primary_flush works on primary. Also check that WAIT FOR
-# triggers an error if called within a function, procedure, anonymous DO block,
-# or inside a transaction with an isolation level higher than READ COMMITTED.
+# on standby. Also check that WAIT FOR triggers an error if called within a
+# function, procedure, anonymous DO block, in a transaction with an isolation
+# level higher than READ COMMITTED, or with an active or registered snapshot.
# Test standby_flush on primary - should error
$node_primary->psql(
@@ -282,12 +282,31 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok($stderr =~
+ /ERROR:\s+WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get an error when running WAIT FOR in a transaction with an isolation level higher than READ COMMITTED"
+);
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
-ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
- "get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
+ok($stderr =~
+ /ERROR:\s+WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED/,
+ "get an isolation-level error when running WAIT FOR after taking a transaction snapshot"
+);
+
+$node_standby->psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL READ COMMITTED; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok($stderr =~
+ /WAIT must be called without an active or registered snapshot/,
+ "get an error when running WAIT FOR with an active or registered snapshot"
);
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
base-commit: 87c6ef14f11705e690a9c54f91e0d97977ce333f
--
2.51.0
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-16 15:50 Nathan Bossart <nathandbossart@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
1 sibling, 1 reply; 36+ messages in thread
From: Nathan Bossart @ 2026-09-16 15:50 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>; aekorotkov@gmail.com
[RMT hat]
Please note that this is now listed as an open item for v19. Alexander,
please give us a status update when you're able.
--
nathan
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-16 23:05 Alexander Korotkov <aekorotkov@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
1 sibling, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-16 23:05 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
Hi!
On Wed, Sep 16, 2026 at 4:28 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> On Sat, Sep 12, 2026 at 10:48 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> >
> > On Fri, Sep 11, 2026 at 11:58 PM Sami Imseih <samimseih.pg@gmail.com> wrote:
> > >
> > > > 1) Could we use 0/0 instead of $lsn3 for these rejection tests? Since
> > > > $lsn3 is deliberately unreachable, the first test can hang if the
> > > > isolation check is missing.
> > >
> > > I think I will keep this as-is. If the isolation-level rejection is
> > > missing, the test is broken.
> > >
> > > > 2) Also, the isolation-error pattern matches the old DETAIL, so
> > > > matching the ERROR: prefix would verify that it is now the primary
> > > > error.
> > >
> > > v4 tightens the new recovery tests so the REPEATABLE READ cases match
> > > the primary ERROR line, rather than the old DETAIL text. I also cleaned
> > > up one test description.
> > >
> > > > The cursor case better additionally check that the misleading
> > > > isolation-level detail is absent.
> > >
> > > I don't think we need that. The cursor case only needs to verify the
> > > new primary error.
> >
> >
> > Thanks. I am OK with your judgement!
>
> Here's a rebase due to a23ab4862cf.
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 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.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v5-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch (5.6K, ../../CAPpHfdv1zWu1ma_SexGLBN_35+cYbJ5SfbdauvjwCPGE-9TyBw@mail.gmail.com/2-v5-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch)
download | inline diff:
From 8fa0e22801e002b3f70e3912b1460951bb034f2c Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 17 Sep 2026 01:54:09 +0300
Subject: [PATCH v5] Say what WAIT's snapshot restriction actually is
The reference page said WAIT "cannot be used in contexts where such a
snapshot must remain active, including transactions running at isolation
levels higher than READ COMMITTED", which reads as a blanket ban at those
levels. It is not one. The rule the code enforces is that no active or
registered snapshot may be held, and a transaction-snapshot-mode
transaction has no snapshot until its first query: WaitStmt is exempt in
PlannedStmtRequiresSnapshot(), so the command itself does not take one.
WAIT therefore works before that first query, which is the order an
application wants anyway, and it is what 447aae13b03 set out to allow by
keeping the wait snapshot-free.
State the rule in terms of the snapshot rather than the isolation level,
and name the two ways to hold one: the level, and an open cursor or an
exported snapshot.
The error for a held snapshot attached the isolation-level explanation
as DETAIL unconditionally, so a READ COMMITTED session with a cursor was
told its isolation level was too high. Emit that DETAIL only when the
isolation level is in fact the reason.
Reported-by: Sami Imseih <samimseih.pg@gmail.com>
Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CAN12%2BYJddgAZzA36JZFX%2BYrnNR7Uubp9qWFz6ASs3v0kL30AMA%40mail.gmail.com
Backpatch-through: 19
---
doc/src/sgml/ref/wait.sgml | 10 ++++++----
src/backend/commands/wait.c | 4 +++-
src/test/recovery/t/049_wait_for_lsn.pl | 21 +++++++++++++++++++++
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/wait.sgml b/doc/src/sgml/ref/wait.sgml
index 9056eb6726b..0b3f3ea13d3 100644
--- a/doc/src/sgml/ref/wait.sgml
+++ b/doc/src/sgml/ref/wait.sgml
@@ -227,10 +227,12 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>'
<para>
<command>WAIT</command> must be executed as a top-level command.
It cannot be executed from a function, procedure, or
- <command>DO</command> 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 <literal>READ COMMITTED</literal>.
+ <command>DO</command> block. It also requires that no active or
+ registered snapshot be held. A transaction running at an isolation
+ level higher than <literal>READ COMMITTED</literal> holds one from its
+ first query until it ends, so <command>WAIT</command> is accepted before
+ that query and rejected after it. An open cursor or an exported
+ snapshot has the same effect at any isolation level.
</para>
<para>
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 2223e91a3e3..e1e98c8c9ea 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -13,6 +13,7 @@
*/
#include "postgres.h"
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -156,7 +157,8 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("WAIT must be called without an active or registered snapshot"),
- errdetail("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ IsolationUsesXactSnapshot() ?
+ errdetail("A transaction running at an isolation level higher than READ COMMITTED holds its snapshot from its first query until it ends.") : 0);
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 3bc936e0f91..df15e5e96fd 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -282,6 +282,15 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+# A transaction-snapshot-mode transaction has no snapshot until its first
+# query, so WAIT FOR is allowed before that point. The target is already
+# replayed here, so the command returns immediately.
+$output = $node_standby->safe_psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn2}'; COMMIT;");
+ok($output eq "success",
+ "WAIT FOR is allowed before the transaction snapshot is taken");
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
@@ -289,6 +298,18 @@ $node_standby->psql(
ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+ok( $stderr =~ /isolation level higher than READ COMMITTED holds its snapshot/,
+ "the isolation level is given as the reason when it is the reason");
+
+# The same error at READ COMMITTED must not blame the isolation level.
+$node_standby->psql(
+ 'postgres',
+ "BEGIN; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
+ "get an error when a cursor holds a snapshot");
+unlike($stderr, qr/isolation level/,
+ "the isolation level is not blamed at READ COMMITTED");
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
# should error
--
2.55.0
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-16 23:06 Alexander Korotkov <aekorotkov@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 0 replies; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-16 23:06 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Sami Imseih <samimseih.pg@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
Hi, Nathan!
On Wed, Sep 16, 2026 at 6:50 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> [RMT hat]
>
> Please note that this is now listed as an open item for v19. Alexander,
> please give us a status update when you're able.
Thank you. I put my hands on it [1]. Will resolve this ASAP.
Links.
1. https://www.postgresql.org/message-id/CAPpHfdv1zWu1ma_SexGLBN_35%2BcYbJ5SfbdauvjwCPGE-9TyBw%40mail.g...
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 01:42 Sami Imseih <samimseih.pg@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-17 01:42 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> 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 ...
but, perhaps it's better to be more flexible here. So, I'm ok with proceeding
with your suggestion.
> 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 ....
```
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 02:03 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 2 replies; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-17 02:03 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 17, 2026 at 9:43 AM Sami Imseih <samimseih.pg@gmail.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.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 02:50 Sami Imseih <samimseih.pg@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
1 sibling, 0 replies; 36+ messages in thread
From: Sami Imseih @ 2026-09-17 02:50 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> > 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?
Because I was thinking that calling WAIT FOR LSN before
BEGIN ISOLATION LEVEL ... is equivalent to calling it after, since the
transaction snapshot is not taken until the first SELECT. However, I can
see how requiring that order could be problematic, especially when the
driver issues BEGIN ISOLATION LEVEL ... automatically.
--
Sami Imseih
Amazon Web Services (AWS).
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 10:13 Alexander Korotkov <aekorotkov@gmail.com>
parent: Xuneng Zhou <xunengzhou@gmail.com>
1 sibling, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-17 10:13 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 17, 2026 at 5:03 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> On Thu, Sep 17, 2026 at 9:43 AM Sami Imseih <samimseih.pg@gmail.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
Attachments:
[application/octet-stream] v6-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch (7.2K, ../../CAPpHfdvRoeM8=FN7GvUay820TG9GSEwP1NorSSLVFJik0HrOqw@mail.gmail.com/2-v6-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch)
download | inline diff:
From a285b3f5e37a57dea72483863a2446bee653daf8 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 17 Sep 2026 01:54:09 +0300
Subject: [PATCH v6] Say what WAIT's snapshot restriction actually is
The reference page said WAIT "cannot be used in contexts where such a
snapshot must remain active, including transactions running at isolation
levels higher than READ COMMITTED", which reads as a blanket ban at those
levels. It is not one. The rule the code enforces is that the current
transaction must hold no snapshot, and a transaction-snapshot-mode
transaction has no snapshot until its first query: WaitStmt is exempt in
PlannedStmtRequiresSnapshot(), so the command itself does not take one.
WAIT therefore works before that first query, which is the order an
application wants anyway, and it is what 447aae13b03 set out to allow by
keeping the wait snapshot-free.
State the rule in terms of the snapshot rather than the isolation level,
name the two ways to hold one, and say why the rule exists at all: a
snapshot held here is what recovery would conflict with while replaying
the records being waited for. Note that the write and flush modes are no
exception, since WAL is recycled only at restartpoints, which cannot pass
a checkpoint record replay has not reached.
The error for a held snapshot named the internal snapshot states instead
of saying what the user did, and attached the isolation-level
explanation as DETAIL unconditionally, so a READ COMMITTED session with a
cursor was told its isolation level was too high. Report what is held
rather than how it is held, and emit the DETAIL only when the isolation
level is in fact the reason.
Reported-by: Sami Imseih <samimseih.pg@gmail.com>
Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Sami Imseih <samimseih.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CAN12%2BYJddgAZzA36JZFX%2BYrnNR7Uubp9qWFz6ASs3v0kL30AMA%40mail.gmail.com
Backpatch-through: 19
---
doc/src/sgml/ref/wait.sgml | 30 +++++++++++++++++++++----
src/backend/commands/wait.c | 6 +++--
src/test/recovery/t/049_wait_for_lsn.pl | 23 ++++++++++++++++++-
3 files changed, 52 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/wait.sgml b/doc/src/sgml/ref/wait.sgml
index cddcbf09695..8ec358f4fd4 100644
--- a/doc/src/sgml/ref/wait.sgml
+++ b/doc/src/sgml/ref/wait.sgml
@@ -260,10 +260,32 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>'
<para>
<command>WAIT</command> must be executed as a top-level command.
It cannot be executed from a function, procedure, or
- <command>DO</command> 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 <literal>READ COMMITTED</literal>.
+ <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, so <command>WAIT</command> is
+ accepted before that query and rejected after it. An open cursor or an
+ exported snapshot has the same effect at any isolation level.
+ </para>
+
+ <para>
+ The restriction above exists because such a snapshot could stand in
+ the way of the very replay being waited for: replaying a record that
+ removes rows the snapshot can still see raises a recovery conflict.
+ This conflict could be resolved by delaying the replay or by cancelling
+ this session. Having to run without a snapshot is also the reason why
+ <command>WAIT</command> is a command rather than a function or a procedure,
+ which execute with one held.
+ </para>
+
+ <para>
+ The other standby modes are not independent of replay either, even though
+ they do not wait for it directly. Their positions advance as WAL arrives,
+ but WAL is recycled only at restartpoints, and a restartpoint cannot be
+ created past a checkpoint record that replay has not reached. While
+ replay is held back, <filename>pg_wal</filename> therefore keeps growing
+ past <xref linkend="guc-max-wal-size"/>, and reception stops once the file
+ system fills.
</para>
<para>
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 2223e91a3e3..86cb27ea2e1 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -13,6 +13,7 @@
*/
#include "postgres.h"
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -155,8 +156,9 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT must be called without an active or registered snapshot"),
- errdetail("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT cannot be executed while the current transaction holds a snapshot"),
+ IsolationUsesXactSnapshot() ?
+ errdetail("This transaction runs at an isolation level higher than READ COMMITTED, so it holds a snapshot from its first query until it ends.") : 0);
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 3bc936e0f91..e9b1dcfc810 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -282,13 +282,34 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+# A transaction-snapshot-mode transaction has no snapshot until its first
+# query, so WAIT FOR is allowed before that point. The target is already
+# replayed here, so the command returns immediately.
+$output = $node_standby->safe_psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn2}'; COMMIT;");
+ok($output eq "success",
+ "WAIT FOR is allowed before the transaction snapshot is taken");
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
-ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+ok( $stderr =~ /isolation level higher than READ COMMITTED, so it holds a snapshot/,
+ "the isolation level is given as the reason when it is the reason");
+
+# The same error at READ COMMITTED must not blame the isolation level.
+$node_standby->psql(
+ 'postgres',
+ "BEGIN; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
+ "get an error when a cursor holds a snapshot");
+unlike($stderr, qr/isolation level/,
+ "the isolation level is not blamed at READ COMMITTED");
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
# should error
--
2.55.0
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 14:11 Alexander Korotkov <aekorotkov@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-17 14:11 UTC (permalink / raw)
To: Xuneng Zhou <xunengzhou@gmail.com>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 17, 2026 at 1:13 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>
> On Thu, Sep 17, 2026 at 5:03 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > On Thu, Sep 17, 2026 at 9:43 AM Sami Imseih <samimseih.pg@gmail.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.
Xuneng, Sami, do you think v6 is good enough?
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 14:53 Sami Imseih <samimseih.pg@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 2 replies; 36+ messages in thread
From: Sami Imseih @ 2026-09-17 14:53 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> > Done in v6.
>
> Xuneng, Sami, do you think v6 is good enough?
I think the user-facing documentation can be simplified considerably while
preserving the important information. I prefer the below changes on top
of v6:
1/ State directly that WAIT cannot be executed while the current transaction
holds a snapshot.
2/ Clarify that WAIT itself does not acquire a snapshot.
3/ Explain that it can therefore be executed before the first snapshot-taking
statement in a REPEATABLE READ or SERIALIZABLE transaction.
4/ Briefly explain why holding a snapshot can prevent a standby wait from
completing.
```
<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, so <command>WAIT</command> is
- accepted before that query and rejected after it. An open cursor or an
- exported snapshot has the same effect at any isolation level.
- </para>
-
- <para>
- The restriction above exists because such a snapshot could stand in
- the way of the very replay being waited for: replaying a record that
- removes rows the snapshot can still see raises a recovery conflict.
- This conflict could be resolved by delaying the replay or by cancelling
- this session. Having to run without a snapshot is also the reason why
- <command>WAIT</command> is a command rather than a function or a procedure,
- which execute with one held.
- </para>
-
- <para>
- The other standby modes are not independent of replay either, even though
- they do not wait for it directly. Their positions advance as WAL arrives,
- but WAL is recycled only at restartpoints, and a restartpoint cannot be
- created past a checkpoint record that replay has not reached. While
- replay is held back, <filename>pg_wal</filename> therefore keeps growing
- past <xref linkend="guc-max-wal-size"/>, and reception stops once the file
- system fills.
+ current transaction holds a snapshot.
+ <command>WAIT</command> itself does not acquire a snapshot, so it can be
+ executed before the first snapshot-taking statement in a
+ <literal>REPEATABLE READ</literal> or <literal>SERIALIZABLE</literal>
+ transaction. Holding a snapshot while using a standby wait mode could
+ delay WAL replay and prevent the wait from completing.
</para>
```
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 15:16 Xuneng Zhou <xunengzhou@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
1 sibling, 0 replies; 36+ messages in thread
From: Xuneng Zhou @ 2026-09-17 15:16 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 17, 2026 at 10:53 PM Sami Imseih <samimseih.pg@gmail.com> wrote:
>
> > > Done in v6.
> >
> > Xuneng, Sami, do you think v6 is good enough?
>
> I think the user-facing documentation can be simplified considerably while
> preserving the important information. I prefer the below changes on top
> of v6:
>
> 1/ State directly that WAIT cannot be executed while the current transaction
> holds a snapshot.
> 2/ Clarify that WAIT itself does not acquire a snapshot.
> 3/ Explain that it can therefore be executed before the first snapshot-taking
> statement in a REPEATABLE READ or SERIALIZABLE transaction.
> 4/ Briefly explain why holding a snapshot can prevent a standby wait from
> completing.
>
Yeah, I think we can simplify and consolidate them. I proposed a
reorganization earlier but it did not do simplification [1]. The
underlying motivation for those constraints stay the same -- to avoid
deadlock/hang while waiting. So I would prefer a small opening para
for consolidation. As for simplification, personally, I am OK with a
detailed explanation because it helps curious users to learn with the
cost of loading some extra bytes. However, the length of the note part
does seem disproportional to that of usage of this command.
[1] https://www.postgresql.org/message-id/CABPTF7UmSuO5HWfBZ+wcJqTAfSFmwAdByrONhWttg5rq56j8vw@mail.gmail...
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 20:52 Alexander Korotkov <aekorotkov@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
1 sibling, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-17 20:52 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 17, 2026 at 5:53 PM Sami Imseih <samimseih.pg@gmail.com> wrote:
>
> > > Done in v6.
> >
> > Xuneng, Sami, do you think v6 is good enough?
>
> I think the user-facing documentation can be simplified considerably while
> preserving the important information. I prefer the below changes on top
> of v6:
>
> 1/ State directly that WAIT cannot be executed while the current transaction
> holds a snapshot.
> 2/ Clarify that WAIT itself does not acquire a snapshot.
> 3/ Explain that it can therefore be executed before the first snapshot-taking
> statement in a REPEATABLE READ or SERIALIZABLE transaction.
> 4/ Briefly explain why holding a snapshot can prevent a standby wait from
> completing.
>
>
> ```
> <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, so <command>WAIT</command> is
> - accepted before that query and rejected after it. An open cursor or an
> - exported snapshot has the same effect at any isolation level.
> - </para>
> -
> - <para>
> - The restriction above exists because such a snapshot could stand in
> - the way of the very replay being waited for: replaying a record that
> - removes rows the snapshot can still see raises a recovery conflict.
> - This conflict could be resolved by delaying the replay or by cancelling
> - this session. Having to run without a snapshot is also the reason why
> - <command>WAIT</command> is a command rather than a function or a procedure,
> - which execute with one held.
> - </para>
> -
> - <para>
> - The other standby modes are not independent of replay either, even though
> - they do not wait for it directly. Their positions advance as WAL arrives,
> - but WAL is recycled only at restartpoints, and a restartpoint cannot be
> - created past a checkpoint record that replay has not reached. While
> - replay is held back, <filename>pg_wal</filename> therefore keeps growing
> - past <xref linkend="guc-max-wal-size"/>, and reception stops once the file
> - system fills.
> + current transaction holds a snapshot.
> + <command>WAIT</command> itself does not acquire a snapshot, so it can be
> + executed before the first snapshot-taking statement in a
> + <literal>REPEATABLE READ</literal> or <literal>SERIALIZABLE</literal>
> + transaction. Holding a snapshot while using a standby wait mode could
> + delay WAL replay and prevent the wait from completing.
> </para>
> ```
Agreed. v7 attached: three paragraphs collapsed into one, close to your
wording. I kept three bits from v6:
* mention the cursor and the exported snapshot as those who may hold snapshots;
* "which standby_replay waits for and the other standby modes
ultimately depend on", so that write and flush are not read as exempt;
* "that is also why WAIT is a command rather than a function or a
procedure", which makes the restriction in the first sentence follow
from the same rule instead of looking arbitrary.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v7-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch (6.4K, ../../CAPpHfdstCVxb-iXsqjZ2umJQdzFud4fypZRCVEZnEOTLyp0gGw@mail.gmail.com/2-v7-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch)
download | inline diff:
From 29aeea300a61495bed9efda5eea88e2696610a93 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 17 Sep 2026 01:54:09 +0300
Subject: [PATCH v7] Say what WAIT's snapshot restriction actually is
The reference page said WAIT "cannot be used in contexts where such a
snapshot must remain active, including transactions running at isolation
levels higher than READ COMMITTED", which reads as a blanket ban at those
levels. It is not one. The rule the code enforces is that the current
transaction must hold no snapshot, and a transaction-snapshot-mode
transaction has no snapshot until its first query: WaitStmt is exempt in
PlannedStmtRequiresSnapshot(), so the command itself does not take one.
WAIT therefore works before that first query, which is the order an
application wants anyway, and it is what 447aae13b03 set out to allow by
keeping the wait snapshot-free.
State the rule in terms of the snapshot rather than the isolation level,
say that WAIT acquires none of its own and name the other ways to hold
one, and give the consequence of holding one: it can delay replay, which
standby_replay waits for and which the other standby modes ultimately
depend on as well.
The error for a held snapshot named the internal snapshot states instead
of saying what the user did, and attached the isolation-level
explanation as DETAIL unconditionally, so a READ COMMITTED session with a
cursor was told its isolation level was too high. Report what is held
rather than how it is held, and emit the DETAIL only when the isolation
level is in fact the reason.
Reported-by: Sami Imseih <samimseih.pg@gmail.com>
Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Sami Imseih <samimseih.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CAN12%2BYJddgAZzA36JZFX%2BYrnNR7Uubp9qWFz6ASs3v0kL30AMA%40mail.gmail.com
Backpatch-through: 19
---
doc/src/sgml/ref/wait.sgml | 14 ++++++++++----
src/backend/commands/wait.c | 6 ++++--
src/test/recovery/t/049_wait_for_lsn.pl | 23 ++++++++++++++++++++++-
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/wait.sgml b/doc/src/sgml/ref/wait.sgml
index cddcbf09695..65af5c32f6e 100644
--- a/doc/src/sgml/ref/wait.sgml
+++ b/doc/src/sgml/ref/wait.sgml
@@ -260,10 +260,16 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>'
<para>
<command>WAIT</command> must be executed as a top-level command.
It cannot be executed from a function, procedure, or
- <command>DO</command> 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 <literal>READ COMMITTED</literal>.
+ <command>DO</command> block. It also cannot be executed while the
+ current transaction holds a snapshot. <command>WAIT</command> itself
+ acquires none, so it can run before the first snapshot-taking statement
+ of a <literal>REPEATABLE READ</literal> or <literal>SERIALIZABLE</literal>
+ transaction, but not after it, and not while a cursor or an exported
+ snapshot holds one at any isolation level. A snapshot held here could
+ delay replay, which <literal>standby_replay</literal> waits for and the
+ other standby modes ultimately depend on. That is also why
+ <command>WAIT</command> is a command rather than a function or a
+ procedure, which execute with one held.
</para>
<para>
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 2223e91a3e3..86cb27ea2e1 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -13,6 +13,7 @@
*/
#include "postgres.h"
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -155,8 +156,9 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT must be called without an active or registered snapshot"),
- errdetail("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT cannot be executed while the current transaction holds a snapshot"),
+ IsolationUsesXactSnapshot() ?
+ errdetail("This transaction runs at an isolation level higher than READ COMMITTED, so it holds a snapshot from its first query until it ends.") : 0);
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 3bc936e0f91..e9b1dcfc810 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -282,13 +282,34 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+# A transaction-snapshot-mode transaction has no snapshot until its first
+# query, so WAIT FOR is allowed before that point. The target is already
+# replayed here, so the command returns immediately.
+$output = $node_standby->safe_psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn2}'; COMMIT;");
+ok($output eq "success",
+ "WAIT FOR is allowed before the transaction snapshot is taken");
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
-ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+ok( $stderr =~ /isolation level higher than READ COMMITTED, so it holds a snapshot/,
+ "the isolation level is given as the reason when it is the reason");
+
+# The same error at READ COMMITTED must not blame the isolation level.
+$node_standby->psql(
+ 'postgres',
+ "BEGIN; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
+ "get an error when a cursor holds a snapshot");
+unlike($stderr, qr/isolation level/,
+ "the isolation level is not blamed at READ COMMITTED");
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
# should error
--
2.55.0
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 21:28 Sami Imseih <samimseih.pg@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-17 21:28 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> Agreed. v7 attached: three paragraphs collapsed into one, close to your
> wording. I kept three bits from v6:
Only nit I have is:
which <literal>standby_replay</literal> waits for and the
+ other standby modes ultimately depend on.
The "ultimately depends on" is not very clear. I would change it to:
"which <literal>standby_replay</literal> waits for, and could also
delay waits in
the other standby modes."
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 23:13 Alexander Korotkov <aekorotkov@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-17 23:13 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 18, 2026 at 12:28 AM Sami Imseih <samimseih.pg@gmail.com> wrote:
> > Agreed. v7 attached: three paragraphs collapsed into one, close to your
> > wording. I kept three bits from v6:
>
> Only nit I have is:
>
> which <literal>standby_replay</literal> waits for and the
> + other standby modes ultimately depend on.
>
> The "ultimately depends on" is not very clear. I would change it to:
>
> "which <literal>standby_replay</literal> waits for, and could also
> delay waits in
> the other standby modes."
That still doesn't sound clear for me. Could you, please, check v8?
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v8-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch (6.4K, ../../CAPpHfdtsGcNmVKFRvi-AGA+C+2Cauu_OHdTsAfLoRp1WW73P5A@mail.gmail.com/2-v8-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch)
download | inline diff:
From 29aeea300a61495bed9efda5eea88e2696610a93 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 17 Sep 2026 01:54:09 +0300
Subject: [PATCH v8] Say what WAIT's snapshot restriction actually is
The reference page said WAIT "cannot be used in contexts where such a
snapshot must remain active, including transactions running at isolation
levels higher than READ COMMITTED", which reads as a blanket ban at those
levels. It is not one. The rule the code enforces is that the current
transaction must hold no snapshot, and a transaction-snapshot-mode
transaction has no snapshot until its first query: WaitStmt is exempt in
PlannedStmtRequiresSnapshot(), so the command itself does not take one.
WAIT therefore works before that first query, which is the order an
application wants anyway, and it is what 447aae13b03 set out to allow by
keeping the wait snapshot-free.
State the rule in terms of the snapshot rather than the isolation level,
say that WAIT acquires none of its own and name the other ways to hold
one, and give the consequence of holding one: it can delay replay, which
standby_replay waits for and which the other standby modes ultimately
depend on as well.
The error for a held snapshot named the internal snapshot states instead
of saying what the user did, and attached the isolation-level
explanation as DETAIL unconditionally, so a READ COMMITTED session with a
cursor was told its isolation level was too high. Report what is held
rather than how it is held, and emit the DETAIL only when the isolation
level is in fact the reason.
Reported-by: Sami Imseih <samimseih.pg@gmail.com>
Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Sami Imseih <samimseih.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CAN12%2BYJddgAZzA36JZFX%2BYrnNR7Uubp9qWFz6ASs3v0kL30AMA%40mail.gmail.com
Backpatch-through: 19
---
doc/src/sgml/ref/wait.sgml | 14 ++++++++++----
src/backend/commands/wait.c | 6 ++++--
src/test/recovery/t/049_wait_for_lsn.pl | 23 ++++++++++++++++++++++-
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/wait.sgml b/doc/src/sgml/ref/wait.sgml
index cddcbf09695..65af5c32f6e 100644
--- a/doc/src/sgml/ref/wait.sgml
+++ b/doc/src/sgml/ref/wait.sgml
@@ -260,10 +260,16 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>'
<para>
<command>WAIT</command> must be executed as a top-level command.
It cannot be executed from a function, procedure, or
- <command>DO</command> 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 <literal>READ COMMITTED</literal>.
+ <command>DO</command> block. It also cannot be executed while the
+ current transaction holds a snapshot. <command>WAIT</command> itself
+ acquires none, so it can run before the first snapshot-taking statement
+ of a <literal>REPEATABLE READ</literal> or <literal>SERIALIZABLE</literal>
+ transaction, but not after it, and not while a cursor or an exported
+ snapshot holds one at any isolation level. A snapshot held here could
+ delay replay, which <literal>standby_replay</literal> waits for and the
+ other standby modes ultimately depend on. That is also why
+ <command>WAIT</command> is a command rather than a function or a
+ procedure, which execute with one held.
</para>
<para>
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 2223e91a3e3..86cb27ea2e1 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -13,6 +13,7 @@
*/
#include "postgres.h"
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -155,8 +156,9 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT must be called without an active or registered snapshot"),
- errdetail("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT cannot be executed while the current transaction holds a snapshot"),
+ IsolationUsesXactSnapshot() ?
+ errdetail("This transaction runs at an isolation level higher than READ COMMITTED, so it holds a snapshot from its first query until it ends.") : 0);
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 3bc936e0f91..e9b1dcfc810 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -282,13 +282,34 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+# A transaction-snapshot-mode transaction has no snapshot until its first
+# query, so WAIT FOR is allowed before that point. The target is already
+# replayed here, so the command returns immediately.
+$output = $node_standby->safe_psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn2}'; COMMIT;");
+ok($output eq "success",
+ "WAIT FOR is allowed before the transaction snapshot is taken");
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
-ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+ok( $stderr =~ /isolation level higher than READ COMMITTED, so it holds a snapshot/,
+ "the isolation level is given as the reason when it is the reason");
+
+# The same error at READ COMMITTED must not blame the isolation level.
+$node_standby->psql(
+ 'postgres',
+ "BEGIN; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
+ "get an error when a cursor holds a snapshot");
+unlike($stderr, qr/isolation level/,
+ "the isolation level is not blamed at READ COMMITTED");
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
# should error
--
2.55.0
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 23:24 Sami Imseih <samimseih.pg@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-17 23:24 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> That still doesn't sound clear for me. Could you, please, check v8?
v7 and v8 are identical it seems, except for version number.
Did you mean to attach something different?
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 23:28 Alexander Korotkov <aekorotkov@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-17 23:28 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 18, 2026 at 2:25 AM Sami Imseih <samimseih.pg@gmail.com> wrote:
> > That still doesn't sound clear for me. Could you, please, check v8?
>
> v7 and v8 are identical it seems, except for version number.
> Did you mean to attach something different?
Sorry for the mess. Please, check v9.
------
Regards,
Alexander Korotkov
Supabase
Attachments:
[application/octet-stream] v9-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch (6.4K, ../../CAPpHfduMFfcVeR=TtwhoCm7yvxU6kp9ZeEGSVt_Bc15dYpN-og@mail.gmail.com/2-v9-0001-Say-what-WAIT-s-snapshot-restriction-actually-is.patch)
download | inline diff:
From 39ee9cd7e774917d2c18630ed40ea55cd9bc3f47 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu, 17 Sep 2026 01:54:09 +0300
Subject: [PATCH v9] Say what WAIT's snapshot restriction actually is
The reference page said WAIT "cannot be used in contexts where such a
snapshot must remain active, including transactions running at isolation
levels higher than READ COMMITTED", which reads as a blanket ban at those
levels. It is not one. The rule the code enforces is that the current
transaction must hold no snapshot, and a transaction-snapshot-mode
transaction has no snapshot until its first query: WaitStmt is exempt in
PlannedStmtRequiresSnapshot(), so the command itself does not take one.
WAIT therefore works before that first query, which is the order an
application wants anyway, and it is what 447aae13b03 set out to allow by
keeping the wait snapshot-free.
State the rule in terms of the snapshot rather than the isolation level,
say that WAIT acquires none of its own and name the other ways to hold
one, and give the consequence of holding one: it can delay replay, which
standby_replay waits for and which the other standby modes ultimately
depend on as well.
The error for a held snapshot named the internal snapshot states instead
of saying what the user did, and attached the isolation-level
explanation as DETAIL unconditionally, so a READ COMMITTED session with a
cursor was told its isolation level was too high. Report what is held
rather than how it is held, and emit the DETAIL only when the isolation
level is in fact the reason.
Reported-by: Sami Imseih <samimseih.pg@gmail.com>
Reported-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Sami Imseih <samimseih.pg@gmail.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Discussion: https://postgr.es/m/CAN12%2BYJddgAZzA36JZFX%2BYrnNR7Uubp9qWFz6ASs3v0kL30AMA%40mail.gmail.com
Backpatch-through: 19
---
doc/src/sgml/ref/wait.sgml | 14 ++++++++++----
src/backend/commands/wait.c | 6 ++++--
src/test/recovery/t/049_wait_for_lsn.pl | 23 ++++++++++++++++++++++-
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/wait.sgml b/doc/src/sgml/ref/wait.sgml
index cddcbf09695..2917309c0d5 100644
--- a/doc/src/sgml/ref/wait.sgml
+++ b/doc/src/sgml/ref/wait.sgml
@@ -260,10 +260,16 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>'
<para>
<command>WAIT</command> must be executed as a top-level command.
It cannot be executed from a function, procedure, or
- <command>DO</command> 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 <literal>READ COMMITTED</literal>.
+ <command>DO</command> block. It also cannot be executed while the
+ current transaction holds a snapshot. <command>WAIT</command> itself
+ acquires none, so it can run before the first snapshot-taking statement
+ of a <literal>REPEATABLE READ</literal> or <literal>SERIALIZABLE</literal>
+ transaction, but not after it, and not while a cursor or an exported
+ snapshot holds one at any isolation level. A snapshot held here could
+ delay replay, which <literal>standby_replay</literal> waits for and which
+ other standby modes can end up waiting for too. That is also why
+ <command>WAIT</command> is a command rather than a function or a
+ procedure, which execute with one held.
</para>
<para>
diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c
index 2223e91a3e3..86cb27ea2e1 100644
--- a/src/backend/commands/wait.c
+++ b/src/backend/commands/wait.c
@@ -13,6 +13,7 @@
*/
#include "postgres.h"
+#include "access/xact.h"
#include "access/xlog.h"
#include "access/xlogrecovery.h"
#include "access/xlogwait.h"
@@ -155,8 +156,9 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
if (HaveRegisteredOrActiveSnapshot())
ereport(ERROR,
errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("WAIT must be called without an active or registered snapshot"),
- errdetail("WAIT cannot be executed within a transaction with an isolation level higher than READ COMMITTED."));
+ errmsg("WAIT cannot be executed while the current transaction holds a snapshot"),
+ IsolationUsesXactSnapshot() ?
+ errdetail("This transaction runs at an isolation level higher than READ COMMITTED, so it holds a snapshot from its first query until it ends.") : 0);
/*
* As the result we should hold no snapshot, and correspondingly our xmin
diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl
index 3bc936e0f91..e9b1dcfc810 100644
--- a/src/test/recovery/t/049_wait_for_lsn.pl
+++ b/src/test/recovery/t/049_wait_for_lsn.pl
@@ -282,13 +282,34 @@ $node_standby->psql(
ok($stderr =~ /recovery is in progress/,
"get an error when running primary_flush on the standby");
+# A transaction-snapshot-mode transaction has no snapshot until its first
+# query, so WAIT FOR is allowed before that point. The target is already
+# replayed here, so the command returns immediately.
+$output = $node_standby->safe_psql(
+ 'postgres',
+ "BEGIN ISOLATION LEVEL REPEATABLE READ; WAIT FOR LSN '${lsn2}'; COMMIT;");
+ok($output eq "success",
+ "WAIT FOR is allowed before the transaction snapshot is taken");
+
$node_standby->psql(
'postgres',
"BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; WAIT FOR LSN '${lsn3}';",
stderr => \$stderr);
-ok( $stderr =~ /WAIT must be called without an active or registered snapshot/,
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
"get an error when running in a transaction with an isolation level higher than REPEATABLE READ"
);
+ok( $stderr =~ /isolation level higher than READ COMMITTED, so it holds a snapshot/,
+ "the isolation level is given as the reason when it is the reason");
+
+# The same error at READ COMMITTED must not blame the isolation level.
+$node_standby->psql(
+ 'postgres',
+ "BEGIN; DECLARE c CURSOR FOR SELECT 1; WAIT FOR LSN '${lsn3}';",
+ stderr => \$stderr);
+ok( $stderr =~ /WAIT cannot be executed while the current transaction holds a snapshot/,
+ "get an error when a cursor holds a snapshot");
+unlike($stderr, qr/isolation level/,
+ "the isolation level is not blamed at READ COMMITTED");
# Test wrapping WAIT FOR into function, procedure, and anonymous DO block --
# should error
--
2.55.0
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-17 23:40 Sami Imseih <samimseih.pg@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Sami Imseih @ 2026-09-17 23:40 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
"and which other standby modes can end up waiting for too."
This works for me. I have no other comments.
--
Sami Imseih
Amazon Web Services (AWS)
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-18 06:48 Alexander Korotkov <aekorotkov@gmail.com>
parent: Sami Imseih <samimseih.pg@gmail.com>
0 siblings, 1 reply; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-18 06:48 UTC (permalink / raw)
To: Sami Imseih <samimseih.pg@gmail.com>; +Cc: Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 18, 2026 at 2:40 AM Sami Imseih <samimseih.pg@gmail.com> wrote:
> "and which other standby modes can end up waiting for too."
>
> This works for me. I have no other comments.
Thank you, pushed.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-18 17:38 Nathan Bossart <nathandbossart@gmail.com>
parent: Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 2 replies; 36+ messages in thread
From: Nathan Bossart @ 2026-09-18 17:38 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 18, 2026 at 09:48:13AM +0300, Alexander Korotkov wrote:
> Thank you, pushed.
Can the open item for this one be marked as resolved?
--
nathan
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-18 17:58 Sami Imseih <samimseih.pg@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 36+ messages in thread
From: Sami Imseih @ 2026-09-18 17:58 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
> On Fri, Sep 18, 2026 at 09:48:13AM +0300, Alexander Korotkov wrote:
> > Thank you, pushed.
>
> Can the open item for this one be marked as resolved?
Done.
--
Sami
^ permalink raw reply [nested|flat] 36+ messages in thread
* Re: Reject WAIT FOR earlier in transaction-snapshot mode
@ 2026-09-18 20:14 Alexander Korotkov <aekorotkov@gmail.com>
parent: Nathan Bossart <nathandbossart@gmail.com>
1 sibling, 0 replies; 36+ messages in thread
From: Alexander Korotkov @ 2026-09-18 20:14 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Sami Imseih <samimseih.pg@gmail.com>; Xuneng Zhou <xunengzhou@gmail.com>; Álvaro Herrera <alvherre@kurilemu.de>; Postgres hackers <pgsql-hackers@lists.postgresql.org>
On Fri, Sep 18, 2026 at 8:38 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Fri, Sep 18, 2026 at 09:48:13AM +0300, Alexander Korotkov wrote:
> > Thank you, pushed.
>
> Can the open item for this one be marked as resolved?
Thank you for reminder. Marked as resolved.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 36+ messages in thread
end of thread, other threads:[~2026-09-18 20:14 UTC | newest]
Thread overview: 36+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-08 19:55 Reject WAIT FOR earlier in transaction-snapshot mode Sami Imseih <samimseih.pg@gmail.com>
2026-09-09 07:13 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-09 07:30 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-09 13:25 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-09 14:00 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-09 14:11 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-09 14:50 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-09 15:41 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-10 01:59 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-09 15:24 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-09-10 11:02 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-10 17:40 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-11 02:24 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-11 15:58 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-12 02:48 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-16 01:28 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-16 15:50 ` Nathan Bossart <nathandbossart@gmail.com>
2026-09-16 23:06 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-16 23:05 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-17 01:42 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-17 02:03 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-17 02:50 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-17 10:13 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-17 14:11 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-17 14:53 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-17 15:16 ` Xuneng Zhou <xunengzhou@gmail.com>
2026-09-17 20:52 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-17 21:28 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-17 23:13 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-17 23:24 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-17 23:28 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-17 23:40 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-18 06:48 ` Alexander Korotkov <aekorotkov@gmail.com>
2026-09-18 17:38 ` Nathan Bossart <nathandbossart@gmail.com>
2026-09-18 17:58 ` Sami Imseih <samimseih.pg@gmail.com>
2026-09-18 20:14 ` Alexander Korotkov <aekorotkov@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox