public inbox for [email protected]
help / color / mirror / Atom feedFrom: Xuneng Zhou <[email protected]>
To: wang.xiao.peng <[email protected]>
Cc: Alexander Korotkov <[email protected]>
Cc: SATYANARAYANA NARLAPURAM <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Re: Bug: WAIT FOR LSN crashes with assertion failure inside PL/pgSQL DO blocks and procedures
Date: Fri, 10 Apr 2026 15:36:04 +0800
Message-ID: <CABPTF7UaXNab-LNyR5jDEu=FpGG_vZSOYqwxzyKrw+60Y5sCJw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAHg+QDcN-n3NUqgRtj=BQb9fFQmH8-DeEROCr=PDbw_BBRKOYA@mail.gmail.com>
<CAPpHfdsZ6YrzX-5uenwvw1VXuG7mpBXSOT6JFTg6aCCC0SaNEg@mail.gmail.com>
<CAHg+QDevycEwWfTMJ6ADZQJR9YSMcsAf=Dg=vH2R8Efcx1NH1w@mail.gmail.com>
<CAPpHfdtJ_oZ3qJvXWAtOV4k02=m1P6AQUgoBofNgytQ3KyotbQ@mail.gmail.com>
<[email protected]>
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
Attachments:
[application/x-patch] v4-0001-Explicitly-forbid-non-top-level-WAIT-FOR-executio.patch (7.1K, 2-v4-0001-Explicitly-forbid-non-top-level-WAIT-FOR-executio.patch)
download | inline diff:
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
view thread (11+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Re: Bug: WAIT FOR LSN crashes with assertion failure inside PL/pgSQL DO blocks and procedures
In-Reply-To: <CABPTF7UaXNab-LNyR5jDEu=FpGG_vZSOYqwxzyKrw+60Y5sCJw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox