Re: [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7 - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7
Date
Msg-id 256115.1751646389@sss.pgh.pa.us
Whole thread Raw
In response to Re: [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7  (Dilip Kumar <dilipbalaut@gmail.com>)
List pgsql-hackers
Dilip Kumar <dilipbalaut@gmail.com> writes:
> On Fri, Jul 4, 2025 at 1:22 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> There's no initplan in the given test case, so I don't see how
>> that idea is going to fix it.  Also, allowing initplans to begin
>> parallelism when the outer query isn't using parallelism seems
>> like it'd be fraught with problems.  It certainly couldn't be
>> something we'd back-patch.

> If you see the original problematic case[1] shown by dipesh, where the
> parallel plan is selected for the initPlan1 but it could not launch
> the worker because of the below check[2] in ExecutePlan.  So here my
> concern was that the number of "max tuple" was passed for the top
> level plan, however the parallelism is restricted for the InitPlan as
> well.

Oh, looking back, I see that indeed the original example happened to
produce a plan that involves an initplan.  But there are plenty of
variants that won't, such as the arguably-more-idiomatic

    return count(*) from test_tab where a between 5.0 and 999999.0;

So I think your emphasis on improving that case is misplaced to start
with.

> If I understand the code comments correctly, they state that "Parallel
> mode only supports complete execution of a plan." Given that
> "InitPlan1" does run to completion, it seems the issue here is that
> when the top-level plan isn't fully executed, it restricts parallelism
> for its sub-plans or init-plans, even if those sub-plans are running
> to completion.

I repeat that I don't think "allow an initplan to run in parallel even
if the outer query can't" is a particularly sane way to tackle this
problem.  For starters, ExecutorRun calls EnterParallelMode() and
ExitParallelMode() globally for the entire query.  We'd have to
rethink how that works and when it would be safe to call those.
Using the query-global estate->es_use_parallel_mode flag wouldn't work
either.  Thought would also be needed about which can't-do-parallelism
restrictions would be safe to override.  I agree that an outer
numberTuples restriction needn't be considered, but I'm not sure that
any others could be ignored.

So on the whole it seems like a research project requiring nontrivial
effort and probably yielding only marginal gains.  It's certainly not
going to yield a back-patchable fix for this performance regression.

            regards, tom lane



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: [HACKERS] pg_upgrade failed with error - ERROR: column "a" in child table must be marked NOT NULL
Next
From: Andres Freund
Date:
Subject: Re: Add progressive backoff to XactLockTableWait functions