On Fri, Apr 10, 2026 at 7:45 AM wang.xiao.peng <[email protected]> wrote: > > > > > > > > > At 2026-04-09 20:28:16, "Alexander Korotkov" <[email protected]> wrote: > >On Thu, Apr 9, 2026 at 10:27 AM SATYANARAYANA NARLAPURAM > ><[email protected]> wrote: > >> On Wed, Apr 8, 2026 at 11:00 PM Alexander Korotkov <[email protected]> > >> wrote: > >>> On Thu, Apr 9, 2026 at 5:03 AM SATYANARAYANA NARLAPURAM > >>> <[email protected]> wrote: > >>> > An assertion failure (server crash in assert-enabled builds) occurs > >>> > when WAIT FOR LSN ... INTO is used inside PL/pgSQL DO blocks or within > >>> > void procedures. > >>> > > >>> > Repro: > >>> > > >>> > -- Run this on a standby > >>> > > >>> > CREATE PROCEDURE test_wait() > >>> > LANGUAGE plpgsql AS $$ > >>> > DECLARE > >>> > result text; > >>> > BEGIN > >>> > WAIT FOR LSN '0/1234' INTO result; > >>> > RAISE NOTICE '%', result; > >>> > END; > >>> > $$; > >>> > CALL test_wait(); > >>> > > >>> > > >>> > The WAIT FOR itself succeeds, but the very next PL/pgSQL statement that > >>> > requires a snapshot crashes the backend with: > >>> > > >>> > TRAP: failed Assert("portal->portalSnapshot == NULL"), > >>> > File: "pquery.c", Line: 1776 > >>> > > >>> > Attached patches for both the test case and a potential fix. Please > >>> > review. > >>> > >>> Thank you for reporting. But I doubt the fix is correct. Even that > >>> this particular might work OK, I don't think it's safe to release > >>> snapshots belonging to functions/procedures: it might affect them. I > >>> tend to think we must forbid wrapping WAIT FOR LSN with > >>> functions/procedures. I'll explore more on this today. > >> > >> > >> Agreed, attached a v2 patch with your suggestion on preventing it running > >> from procedures. > > > >Thank you. I've slightly revised your patch. I'm going to push it if > >no objections. > > > >------ > >Regards, > >Alexander Korotkov > >Supabase > > Hi, > This patch looks good to me overall. I spotted a typo in the commit message: > > "it could pass this check causing an error elsewhere. This commit implments" > > implments -> implements, missing an "e". >
I’ve revised the patch. Moving the non–top-level rejection to the beginning of the function may help avoid unnecessary parsing and validation work, although it could make the reasoning slightly less localized. Since this is user-facing, should we explicitly document this constraint to make the behavior less surprising? The rejection applies not only to wrapping the command in a procedure or function, but also within a DO block. It might also be worth adding a regression test and refining the error message accordingly. With this new constraint, some existing comments were outdated and have been updated as well. -- Best, Xuneng
From c98351d28b158fc0b34e60a16639d35e7f358f72 Mon Sep 17 00:00:00 2001 From: alterego655 <[email protected]> Date: Fri, 10 Apr 2026 14:46:32 +0800 Subject: [PATCH v4] Explicitly forbid non-top-level WAIT FOR execution Previously we were relying on a snapshot-based check to detect invalid execution contexts. However, when WAIT FOR is wrapped into a stored procedure or a DO block, it could pass this check, causing an error elsewhere. This commit implements an explicit isTopLevel check to reject WAIT FOR when called from within a function, procedure, or DO block. The isTopLevel check catches these cases early with a clear error message, matching the pattern used by other utility commands like VACUUM and REINDEX. The snapshot check is retained for the remaining case: top-level execution within a transaction block using an isolation level higher than READ COMMITTED. Also adds tests for WAIT FOR LSN wrapped in a procedure and DO block, complementing the existing test that uses a function wrapper. Reported-by: Satyanarayana Narlapuram <[email protected]> Discussion: https://postgr.es/m/CAHg%2BQDcN-n3NUqgRtj%3DBQb9fFQmH8-DeEROCr%3DPDbw_BBRKOYA%40mail.gmail.com Author: Satyanarayana Narlapuram <[email protected]> Reviewed-by: Alexander Korotkov <[email protected]> --- doc/src/sgml/ref/wait_for.sgml | 8 +++++++ src/backend/commands/wait.c | 22 +++++++++++++----- src/backend/tcop/utility.c | 3 ++- src/include/commands/wait.h | 3 ++- src/test/recovery/t/049_wait_for_lsn.pl | 30 ++++++++++++++++++++++--- 5 files changed, 55 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/ref/wait_for.sgml b/doc/src/sgml/ref/wait_for.sgml index c30fba6f05a..9ba785ea321 100644 --- a/doc/src/sgml/ref/wait_for.sgml +++ b/doc/src/sgml/ref/wait_for.sgml @@ -221,6 +221,14 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>' <refsect1> <title>Notes</title> + <para> + <command>WAIT FOR</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>. + </para> <para> <command>WAIT FOR</command> waits until the specified diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c index 85fcd463b4c..12888281d9b 100644 --- a/src/backend/commands/wait.c +++ b/src/backend/commands/wait.c @@ -31,7 +31,8 @@ void -ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, DestReceiver *dest) +ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, + DestReceiver *dest) { XLogRecPtr lsn; int64 timeout = 0; @@ -45,6 +46,16 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, DestReceiver *dest) bool no_throw_specified = false; bool mode_specified = false; + /* + * 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"))); + /* Parse and validate the mandatory LSN */ lsn = DatumGetLSN(DirectFunctionCall1(pg_lsn_in, CStringGetDatum(stmt->lsn_literal))); @@ -142,10 +153,9 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, DestReceiver *dest) * implying a kind of self-deadlock. This is the reason why WAIT FOR is a * command, not a procedure or function. * - * At first, we should check there is no active snapshot. According to - * PlannedStmtRequiresSnapshot(), even in an atomic context, CallStmt is - * processed with a snapshot. Thankfully, we can pop this snapshot, - * because PortalRunUtility() can tolerate this. + * Non-top-level contexts are rejected above, but be defensive and pop + * any active snapshot if one is present. PortalRunUtility() can + * tolerate utility commands that remove the active snapshot. */ if (ActiveSnapshotSet()) PopActiveSnapshot(); @@ -161,7 +171,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, DestReceiver *dest) 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 from a function or procedure, nor within a transaction with an isolation level higher than READ COMMITTED.")); + errdetail("WAIT FOR cannot be executed within a transaction with an isolation level higher than READ COMMITTED.")); /* * As the result we should hold no snapshot, and correspondingly our xmin diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 1d34c19913e..73a56f1df1d 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -1062,7 +1062,8 @@ standard_ProcessUtility(PlannedStmt *pstmt, case T_WaitStmt: { - ExecWaitStmt(pstate, (WaitStmt *) parsetree, dest); + ExecWaitStmt(pstate, (WaitStmt *) parsetree, isTopLevel, + dest); } break; diff --git a/src/include/commands/wait.h b/src/include/commands/wait.h index 521a312908d..a563579695c 100644 --- a/src/include/commands/wait.h +++ b/src/include/commands/wait.h @@ -16,7 +16,8 @@ #include "parser/parse_node.h" #include "tcop/dest.h" -extern void ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, DestReceiver *dest); +extern void ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, + DestReceiver *dest); extern TupleDesc WaitStmtResultDesc(WaitStmt *stmt); #endif /* WAIT_H */ diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl index bf61b8c47cf..8358c57f7b7 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -215,9 +215,33 @@ $node_standby->psql( 'postgres', "SELECT pg_wal_replay_wait_wrap('${lsn3}');", stderr => \$stderr); -ok( $stderr =~ - /WAIT FOR must be called without an active or registered snapshot/, - "get an error when running within another function"); +ok($stderr =~ /WAIT FOR can only be executed as a top-level statement/, + "get an error when running within a function"); + +$node_primary->safe_psql( + 'postgres', qq[ +CREATE PROCEDURE pg_wal_replay_wait_proc(target_lsn pg_lsn) AS \$\$ + BEGIN + EXECUTE format('WAIT FOR LSN %L;', target_lsn); + END +\$\$ +LANGUAGE plpgsql; +]); + +$node_primary->wait_for_catchup($node_standby); +$node_standby->psql( + 'postgres', + "CALL pg_wal_replay_wait_proc('${lsn3}');", + stderr => \$stderr); +ok($stderr =~ /WAIT FOR can only be executed as a top-level statement/, + "get an error when running within a procedure"); + +$node_standby->psql( + 'postgres', + "DO \$\$ BEGIN EXECUTE format('WAIT FOR LSN %L;', '${lsn3}'); END \$\$;", + stderr => \$stderr); +ok($stderr =~ /WAIT FOR can only be executed as a top-level statement/, + "get an error when running within a DO block"); # 6. Check parameter validation error cases on standby before promotion my $test_lsn = -- 2.51.0
