| From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com> |
| Cc: | Amit Langote <amitlangote09(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Subject: | Re: unnecessary executor overheads around seqscans |
| Date: | 2026-01-25 13:20:20 |
| Message-ID: | CAApHDvriDesugxt4oi4ePp-kvHpMojpYXk46AFhXFO-b6uH-8w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, 25 Jan 2026 at 04:36, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2026-01-24 19:36:08 +1300, David Rowley wrote:
> > 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;
> > - }
>
> What about the "if we're serially executing a scan that was planned to be
> parallel." part? I frankly don't really know what precisely was referencing...
I think that comment is wrong and that maybe it worked that way at
some point when Parallel Seq Scan was in development, but it doesn't
seem to work that way today, and does not appear to have when Parallel
Seq Scan was committed. It's pretty easy to see that code isn't
triggered just by setting max_parallel_workers to 0 and running query
which triggers a Parallel Seq Scan. I think that's what "serially
executing a scan that was planned to be parallel" means, right? The
debug_parallel_query = on case uses a non-parallel Seq scan.
One user-visible side effect of initialising the TableScanDesc during
plan initialisation rather than on the first row is that the stats
will record the Seq Scan even if it's never executed in the plan.
Currently what we do there is somewhat inconsistent as with a Parallel
Seq Scan we'll count the seq scan stat if the Gather node is executed,
regardless of if the Seq Scan node gets executed: ExecGather ->
ExecInitParallelPlan -> ExecParallelInitializeDSM ->
ExecSeqScanInitializeDSM -> table_beginscan_parallel. But with a
non-parallel Seq Scan, the scan will be tracked after fetching the
first row.
Added Robert to see if he remembers about the comment or can shed some
light on it.
David
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jelte Fennema-Nio | 2026-01-25 14:35:42 | Re: RFC: Allow EXPLAIN to Output Page Fault Information |
| Previous Message | Julien Rouhaud | 2026-01-25 12:57:31 | Re: Cleaning up PREPARE query strings? |