Re: Streaming decoding fails with "unexpected table_index_fetch_tuple call during logical decoding" when a relation has a TOASTed conbin (follow-up to BUG #18641)

On Thu, 29 Oct 2026 at 10:22, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Hi Chao,
>
> > I agree with the solution. However, I have a minor concern about the naming.
> > BeginSystemTableScan() and EndSystemTableScan() sound like functions that
> > actually start and end system table scans, whereas they only track sysscan_depth.
> > How about renaming them to TrackSystemTableScanStart() and
> > TrackSystemTableScanEnd() to better reflect their purpose?
>
> +1, PSA new version.
>
Hi Kuroda-san,

I have reviewed and tested the patches.
I am able to reproduce the issue in PG14 to PG18 using the test added
in stream.sql and confirm that the corresponding patches address the
issue.
Patches is applying cleanly on all branches and passed 'make check-world'

I have some minor comments:
1. For all patches: Most of the comments for function in the file
'xact.c' is like of format:
/*
* Function name
*
* Function Description
*/
Maybe we should follow the same for the functions added by these patches?

2. For patches for PG14 to PG18: In logical.c we can remove:
+#include "access/genam.h"
#include "access/xact.h"

In logical.c we only introduced "ResetSystemTableScanState();" and
this function which was declared 'access/xact.h'
logical.c already includes access/xact.h

3. For patches for PG 18 and PG 17: I noticed a unrelated line deletion change:
#include "utils/snapmgr.h"

-
/* ----------------------------------------------------------------

Was it intentional?

4. For patches for PG 14 and PG 15:
+ ALTER TABLE nested_sys_scan_test ALTER COLUMN checked_value set
storage external;
I think we should capitalize 'set storage external;'

Thanks,
Shlok Kyal