Subject: Re: unnecessary executor overheads around seqscans

From: Amit Langote <[email-masked]>
Date: Sat, 24 Jan 2026 15:21:22 +0900

Hi,

On Sat, Jan 24, 2026 at 5:16 AM Andres Freund <[email-masked]> wrote:
>
> Hi,
>
> In [1] I was looking at the profile of a seqscan with a where clause that
> doesn't match any of the many rows.  I was a bit saddened by where we were
> spending time.
>
>
> - The fetching of variables, as well as the null check of scandesc, in
>   SeqNext() is repeated in every loop iteration of ExecScanExtended, despite
>   that obviously not being required after the first iteration
>
>   We could perhaps address this by moving the check to the callers of
>   ExecScanExtended() or by extending ExecScanExtended to have an explicit
>   beginscan callback that it calls after.
>
>   Or perhaps we could just make it so that the entire if (scandesc == NULL)
>   branch isn't needed?

Kind of like ExecProcNodeFirst(), what if we replace the variant
selection in ExecInitSeqScan() with just:

    scanstate->ss.ps.ExecProcNode = ExecSeqScanFirst;

ExecSeqScanFirst() would:

- do the table_beginscan() call that's currently inside the if
(scandesc == NULL) block in SeqNext()

- select and install the appropriate ExecSeqScan variant based on
qual/projection/EPQ

- call that variant to fetch and return the first tuple.

Then we can just remove the if (scandesc == NULL) block from SeqNext() entirely.

> - 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?

Haven't thought about this.

> - 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;

I assume this relies on table_scan_getnextslot() being inlined into
ExecScanExtended()?

> - We repeatedly store the table oid in the slot, table_scan_getnextslot() and
>   then again in ExecStoreBufferHeapTuple(). This shows up in the profile.
>
>   I wish we had made the slot a property of the scan, that way the scan could
>   assume the slot already has the oid set...

I've noticed this when working on my batching patch. I set
tts_tableOid when creating the slots used in the batch themselves, so
the per-tuple assignment isn't needed.

> - 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.

-- 
Thanks, Amit Langote

---

From: David Rowley <[email-masked]>
Date: Sat, 24 Jan 2026 19:36:08 +1300

> selection in ExecInitSeqScan() with just:

I imagined moving it to ExecInitSeqScan() and just avoid doing it when
we're doing EXPLAIN or we're doing a parallel scan. Something like the
attached, which is giving me a 4% speedup selecting from a million row
table with a single int column running a seqscan query with a WHERE
clause matching no rows.

> like the simpler option, unless I misunderstood.

It's probably too late to change it now, but wouldn't it have been
better if scan_getnextslot had been coded to return the TupleTableSlot
rather than bool? That way you could get the sibling call in
ExecStoreBufferHeapTuple() and in SeqNext().

I also noticed my compiler does not inline SeqNext(). Adding a
pg_attribute_always_inline results in it getting inlined and gives a
small speedup.

David
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
index b8119face43..87420e60dc9 100644
--- a/src/backend/executor/nodeSeqscan.c
+++ b/src/backend/executor/nodeSeqscan.c
@@ -63,17 +63,6 @@ SeqNext(SeqScanState *node)
 	direction = estate->es_direction;
 	slot = node->ss.ss_ScanTupleSlot;
 
-	if (scandesc == NULL)
-	{
-		/*
-		 * We reach here if the scan is not parallel, or if we're serially
-		 * executing a scan that was planned to be parallel.
-		 */
-		scandesc = table_beginscan(node->ss.ss_currentRelation,
-								   estate->es_snapshot,
-								   0, NULL);
-		node->ss.ss_currentScanDesc = scandesc;
-	}
 
 	/*
 	 * get the next tuple from the table
@@ -258,6 +247,21 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags)
 	scanstate->ss.ps.qual =
 		ExecInitQual(node->scan.plan.qual, (PlanState *) scanstate);
 
+	/*
+	 * Build the TableScanDesc unless we're just doing an EXPLAIN without
+	 * ANALYZE.  Parallel SeqScan's TableScanDesc is built by
+	 * ExecSeqScanInitializeDSM or ExecSeqScanInitializeWorker.
+	 */
+	if ((eflags & EXEC_FLAG_EXPLAIN_ONLY) == 0 &&
+		node->scan.plan.parallel_aware == false)
+	{
+		scanstate->ss.ss_currentScanDesc =
+			table_beginscan(scanstate->ss.ss_currentRelation,
+							estate->es_snapshot,
+							0,
+							NULL);
+	}
+
 	/*
 	 * When EvalPlanQual() is not in use, assign ExecProcNode for this node
 	 * based on the presence of qual and projection. Each ExecSeqScan*()