Re: REPACK (CONCURRENTLY) can crash a logical decoding session

On Wed, 2 Sept 2026 at 19:19, Antonin Houska <ah(at)cybertec(dot)at> wrote:
>
> Thom Brown <thom(at)linux(dot)com> wrote:
>
> > I have been test-driving repack in an attempt to break it. I had no
> > luck, but I set Claude on a mission, and it reported the following.
>
> TBH I usually fail to follow the "analysis" of LLMs (I found it rather
> chaotic). Nevertheless, what you posted pointed my attention to an obvious
> failure to pass the correct options to heap_toast_insert_or_update():
>
> > 1) The catch-up phase's TOAST rows are still logically logged.
> >
> > heap_update() derives walLogical from TABLE_UPDATE_NO_LOGICAL and honours
> > it for the main tuple, but the TOAST call underneath passes a hardcoded
> > 0 rather than the caller's options (heapam.c:3965):
> >
> > if (need_toast)
> > {
> > /* Note we always use WAL and FSM during updates */
> > heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0);
> >
>
> Attached (0001) is a spec file for the isolation tester that reproduces the
> crash reliably. It's a separate diff because I'm not sure it needs to be
> merged.
>
> This appears to be true - a special case that I have missed:
>
> > The crash needs an output plugin that sets
> > OutputPluginOptions.receive_rewrites.
>
> > Fixes
> > -----
> >
> > Either change alone stops the crash, but both look worth making.
>
> > For (1), just propagate the caller's options as the insert path does:
> >
> > - heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup, 0);
> > + heaptup = heap_toast_insert_or_update(relation, newtup, &oldtup,
> > + options);
>
> This is not true. I didn't check (2), but (1) is wrong. The correct fix is
> attached (0002).
>
> Thanks a lot for your testing!

Thanks for taking a look. I have tested your fix and it no longer
crashes with the test case, so you appear to have resolved the
problem.

Regards

Thom