| From: | Jiří Kavalík <jiri(dot)kavalik(at)comgate(dot)cz> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
| Cc: | pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
| Subject: | 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) |
| Date: | 2026-10-06 09:33:40 |
| Message-ID: | CAF7a2M9tnu4P+Hcbbu45R+GZnmqnLmi=D7k7sQLF3GJjQRPR3Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Hi Kuroda-san,
> Hmm, I'm not excited to modify the exposed data structure yet, unless
there is a
> real issue.
Agreed. It was only a readability suggestion, not a fix for a real failure.
I checked again. CheckXidAlive is set only at the start of each change in
ReorderBufferProcessTXN(), before that change's catalog access. It is
cleared
only after the change loop, or on abort, where ResetLogicalStreamingState()
also resets the new depth counter. So it does not change while a scan is
open,
except on the error path, which v1 already handles. v1 looks complete to me.
Thanks for the patch.
Regards,
Jiří Kavalík
út 6. 10. 2026 v 11:22 odesílatel Hayato Kuroda (Fujitsu) <
kuroda(dot)hayato(at)fujitsu(dot)com> napsal:
> Hi Jiří,
>
> Thanks for the test. I understood that no issues were found for now.
>
> > One question while reading the patch, not a problem I could trigger: the
> depth
> > is incremented in systable_beginscan* and decremented in
> systable_endscan* only
> > if CheckXidAlive is valid, and that is evaluated separately at each end.
> If
> > CheckXidAlive changed while a scan was open, the counter would be off by
> one.
> > I could not find a path where that happens. Error paths look fine, since
> > AbortTransaction/AbortSubTransaction call ResetLogicalStreamingState().
> If a
> > SysScanDesc field is acceptable despite the header concern, remembering
> in the
> > scan whether it was counted would make the pairing explicit.
>
> Hmm, I'm not excited to modify the exposed data structure yet, unless
> there is a
> real issue. Per my analysis, SysScanDescData only contains pointers (8
> bytes),
> it does not have any paddings. This meant we need to modify a size of the
> data
> structure, it might cause failures somewhere.
> Also, the existing code has the same possibility while turning on/off
> bsysscan,
> right? So I feel it's already accepted.
> (Of course, we must fix if it causes a real failure)
>
> Best regards,
> Hayato Kuroda
> FUJITSU LIMITED
>
>
--
S pozdravem
Jiří Kavalík
jiri(dot)kavalik(at)comgate(dot)cz
Comgate a.s.
Gočárova třída 1754/48b, 500 02 Hradec Králové
www.comgate.cz
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2026-10-06 20:06:47 | Re: BUG #19747: pg_dump does not pin array_nulls, so restore mangles NULL array elements |
| Previous Message | Hayato Kuroda (Fujitsu) | 2026-10-06 09:21:59 | 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) |