Hi, On 2026-01-24 15:21:22 +0900, Amit Langote wrote: > On Sat, Jan 24, 2026 at 5:16 AM Andres Freund <[email protected]> wrote: > > - The TupIsNull(slot) check in ExecScanExtended() is redundant with the > > return > > value of table_scan_getnextslot(), but the compiler doesn't grok that. > > > > We can use a pg_assume() in table_scan_getnextslot() to make the compiler > > understand. > > Something like this? > > result = sscan->rs_rd->rd_tableam->scan_getnextslot(sscan, direction, > slot); > pg_assume(result == !TupIsNull(slot)); > return result;
Yep, that seems to clue at least gcc into understanding the situation. I only tried two pg_assume(!found || !TupIsNull(slot)), but yours should probably also work. > I assume this relies on table_scan_getnextslot() being inlined into > ExecScanExtended()? Yes. But I think that that's a good bet, given that it's an inline function and only used once in nodeSeqscan.c. > > - heap_getnextslot() calls ExecStoreBufferHeapTuple() and then returns > > true. That prevents the sibiling call optimization. > > > > We should change ExecStoreBufferHeapTuple() to return true. Nobody uses > > the > > current return value. Alternatively we should consider just moving it to > > somewhere heapam.c/heapam_handler.c can see the implementations, they're > > the > > only ones that should use it anyway. > > Makes sense. Changing ExecStoreBufferHeapTuple() to return true seems > like the simpler option, unless I misunderstood. Yea. There's probably more to be gained by the other approach, but it's also somewhat painful due to some functionality being private to execTuples.c right now. Greetings, Andres Freund
