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