Re: Re: Bug: WAIT FOR LSN crashes with assertion failure inside PL/pgSQL DO blocks and procedures

On Fri, Apr 10, 2026 at 7:45 AM wang.xiao.peng <wxp_728(at)163(dot)com> wrote:
>
>
>
>
>
>
>
>
> At 2026-04-09 20:28:16, "Alexander Korotkov" <aekorotkov(at)gmail(dot)com> wrote:
> >On Thu, Apr 9, 2026 at 10:27 AM SATYANARAYANA NARLAPURAM
> ><satyanarlapuram(at)gmail(dot)com> wrote:
> >> On Wed, Apr 8, 2026 at 11:00 PM Alexander Korotkov <aekorotkov(at)gmail(dot)com> wrote:
> >>> On Thu, Apr 9, 2026 at 5:03 AM SATYANARAYANA NARLAPURAM
> >>> <satyanarlapuram(at)gmail(dot)com> 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