On Wed, 2 Sept 2026 at 19:19, Antonin Houska <[email protected]> wrote: > > Thom Brown <[email protected]> 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
