On Sat, Jan 24, 2026 at 9:01 PM Andres Freund <andres@anarazel.de> wrote:
>
> On 2026-01-24 15:23:44 +0530, Amit Kapila wrote:
> > On Sat, Jan 24, 2026 at 1:46 AM Andres Freund <andres@anarazel.de> wrote:
> > >
> > > - The checkXidAlive checks that have been added to table_scan_getnextslot()
> > > show up noticeably and in every loop iteration, despite afaict never being reachable
> > >
> > > It's not obvious to me that this should
> > > a) be in table_scan_getnextslot(), rather than in beginscan - how could it
> > > change in the middle of a scan? That would require a wrapper around
> > > rd_tableam->scan_begin(), but that seems like it might be good anyway.
> > > b) not just be an assertion?
> > >
> >
> > IIRC, the main reason for having this precautionary check in the API
> > is to ensure that during logical decoding we never access the table AM
> > or
> > heap APIs directly when scanning catalog tables. This restriction
> > exists because we only check for concurrent aborts inside the
> > systable_* APIs.
>
> I know why the check exists - but why does it have to be in
> table_scan_getnextslot(), which is executed very frequently, rather than
> table_beginscan*(), which is executed much less frequently.
>
I thought about this point and couldn't think of any reason why this
check can't be in table_beginscan*(). I think your idea of having a
wrapper around scan_begin() to handle this check is a good one.
--
With Regards,
Amit Kapila.
