On Sat, Jan 24, 2026 at 9:01 PM Andres Freund <[email protected]> wrote: > > On 2026-01-24 15:23:44 +0530, Amit Kapila wrote: > > On Sat, Jan 24, 2026 at 1:46 AM Andres Freund <[email protected]> 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.
