Inadequate parallel-safety check for SubPlans

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)postgreSQL(dot)org
Subject: Inadequate parallel-safety check for SubPlans
Date: 2017-04-12 18:41:09
Message-ID: 7064.1492022469@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While poking at the question of parallel_safe marking for Plans,
I noticed that max_parallel_hazard_walker() does this:

/* We can push the subplans only if they are parallel-safe. */
else if (IsA(node, SubPlan))
return !((SubPlan *) node)->parallel_safe;

This is 100% wrong. It's failing to recurse into the subexpressions of
the SubPlan, which could very easily include parallel-unsafe function
calls. Example:

regression=# explain select * from tenk1 where int4(unique1 + random()) not in (select ten from tenk2);
QUERY PLAN
-----------------------------------------------------------------------------
Gather (cost=470.00..884.25 rows=5000 width=244)
Workers Planned: 4
-> Parallel Seq Scan on tenk1 (cost=470.00..884.25 rows=1250 width=244)
Filter: (NOT (hashed SubPlan 1))
SubPlan 1
-> Seq Scan on tenk2 (cost=0.00..445.00 rows=10000 width=4)

random() is parallel-restricted so it's not cool that the SubPlan was
allowed to be passed down to workers.

I tried to fix it like this:

else if (IsA(node, SubPlan))
{
if (!((SubPlan *) node)->parallel_safe &&
max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
}

but got some failures in the regression tests due to things not getting
parallelized when expected. Poking into that, I remembered that for some
SubPlans, the testexpr contains Params representing the output columns
of the sub-select. So the recursive call of max_parallel_hazard_walker
visits those and deems the expression parallel-restricted.

Basically, therefore, this logic is totally inadequate. I think what
we need to do is improve matters so that while looking at the testexpr
of a SubPlan, we pass down a whitelist saying that the Params having
the numbers indicated by the SubLink's paramIds list are OK.

I'm also suspicious that the existing dumb treatment of Params here
may be blocking recognition that correlated subplans are parallel-safe.
But that would only be a failure to apply a possible optimization,
not a failure to generate a valid plan.

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-04-12 18:41:34 Re: Adding support for Default partition in partitioning
Previous Message Robert Haas 2017-04-12 18:39:11 Re: Adding support for Default partition in partitioning