| From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
|---|---|
| To: | Sami Imseih <samimseih(dot)pg(at)gmail(dot)com> |
| Cc: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Reject WAIT FOR earlier in transaction-snapshot mode |
| Date: | 2026-09-09 07:30:22 |
| Message-ID: | CABPTF7UNdy3fDh-gHqmA0-rjymT35hzusWb-364i23UUHybA9g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Sep 9, 2026 at 3:13 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>
> Hi Sami,
>
> On Wed, Sep 9, 2026 at 3:55 AM Sami Imseih <samimseih(dot)pg(at)gmail(dot)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.
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | vignesh C | 2026-09-09 07:28:40 | Re: Include sequences in publications created by pg_createsubscriber |