Re: [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7

Dilip Kumar <dilipbalaut@gmail.com> writes:
> On Tue, May 20, 2025 at 1:45 PM DIPESH DHAMELIYA
> <dipeshdhameliya125@gmail.com> wrote:
>> I understand but aren't we blocking parallelism for genuine cases with
>> a very complex query where parallelism can help to some extent to
>> improve execution time? Users can always rewrite a query (for example
>> using TOP clause) if they are expecting one tuple to be returned.

> IMHO, you are targeting the fix at the wrong place.  Basically if we
> accept this fix means the already existing functions for the users
> will start performing bad for enabling the parallelism in some other
> cases where they will see benefits, so it might not be acceptable by
> many users to change the application and rewrite all the procedures to
> get the same performance they were getting earlier.

I noticed this patch in the commitfest.  I agree it's a bit
unfortunate that that bug fix disabled parallelism for queries issued
via exec_eval_expr.  There isn't a lot of choice if we retain
the maxtuples = 2 setting, since (as noted in the thread leading up
to that commit) the executor can't safely use parallelism alongside
a return-tuple-limit setting.  So it was outright unsafe before.

However ... I'm not sure I buy the argument that removing the
maxtuples = 2 bit will be problematic for performance.  If the query
returns more than one tuple then exec_eval_expr is going to throw
an error, so do we really need to optimize getting to that error?
Especially at the cost of pessimizing other, actually-useful cases?

As for the patch itself, I'm not sure about removing the maxtuples
argument altogether, since it seems to me that we might still have
some use for maxtuples limits in plpgsql in future.  A one-liner
change to make exec_eval_expr pass 0 seems sufficient.

> I would not say that your concern is wrong because for internal
> aggregate initplan we are processing all the tuple so logically it
> should use the parallel plan, so IMHO we need to target the fix for
> enabling the parallelism for initplan in cases where outer query has
> input the max number of tuple because that limit is for the outer plan
> not for the initplan.

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.

			regards, tom lane