1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-08 11:42:09 +03:00

Fix testing of parallel-safety of SubPlans.

is_parallel_safe() supposed that the only relevant property of a SubPlan
was the parallel safety of the referenced subplan tree.  This is wrong:
the testexpr or args subtrees might contain parallel-unsafe stuff, as
demonstrated by the test case added here.  However, just recursing into the
subtrees fails in a different way: we'll typically find PARAM_EXEC Params
representing the subplan's output columns in the testexpr.  The previous
coding supposed that any Param must be treated as parallel-restricted, so
that a naive attempt at fixing this disabled parallel pushdown of SubPlans
altogether.  We must instead determine, for any visited Param, whether it
is one that would be computed by a surrounding SubPlan node; if so, it's
safe to push down along with the SubPlan node.

We might later be able to extend this logic to cope with Params used for
correlated subplans and other cases; but that's a task for v11 or beyond.

Tom Lane and Amit Kapila

Discussion: https://postgr.es/m/7064.1492022469@sss.pgh.pa.us
This commit is contained in:
Tom Lane
2017-04-18 15:43:56 -04:00
parent 539f67012e
commit 39151781c8
4 changed files with 57 additions and 6 deletions

View File

@ -93,6 +93,7 @@ typedef struct
{
char max_hazard; /* worst proparallel hazard found so far */
char max_interesting; /* worst proparallel hazard of interest */
List *safe_param_ids; /* PARAM_EXEC Param IDs to treat as safe */
} max_parallel_hazard_context;
static bool contain_agg_clause_walker(Node *node, void *context);
@ -1056,6 +1057,7 @@ max_parallel_hazard(Query *parse)
context.max_hazard = PROPARALLEL_SAFE;
context.max_interesting = PROPARALLEL_UNSAFE;
context.safe_param_ids = NIL;
(void) max_parallel_hazard_walker((Node *) parse, &context);
return context.max_hazard;
}
@ -1084,6 +1086,7 @@ is_parallel_safe(PlannerInfo *root, Node *node)
/* Else use max_parallel_hazard's search logic, but stop on RESTRICTED */
context.max_hazard = PROPARALLEL_SAFE;
context.max_interesting = PROPARALLEL_RESTRICTED;
context.safe_param_ids = NIL;
return !max_parallel_hazard_walker(node, &context);
}
@ -1171,18 +1174,49 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context)
return true;
}
/* We can push the subplans only if they are parallel-safe. */
/*
* Only parallel-safe SubPlans can be sent to workers. Within the
* testexpr of the SubPlan, Params representing the output columns of the
* subplan can be treated as parallel-safe, so temporarily add their IDs
* to the safe_param_ids list while examining the testexpr.
*/
else if (IsA(node, SubPlan))
return !((SubPlan *) node)->parallel_safe;
{
SubPlan *subplan = (SubPlan *) node;
List *save_safe_param_ids;
if (!subplan->parallel_safe &&
max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
save_safe_param_ids = context->safe_param_ids;
context->safe_param_ids = list_concat(list_copy(subplan->paramIds),
context->safe_param_ids);
if (max_parallel_hazard_walker(subplan->testexpr, context))
return true; /* no need to restore safe_param_ids */
context->safe_param_ids = save_safe_param_ids;
/* we must also check args, but no special Param treatment there */
if (max_parallel_hazard_walker((Node *) subplan->args, context))
return true;
/* don't want to recurse normally, so we're done */
return false;
}
/*
* We can't pass Params to workers at the moment either, so they are also
* parallel-restricted.
* parallel-restricted, unless they are PARAM_EXEC Params listed in
* safe_param_ids, meaning they could be generated within the worker.
*/
else if (IsA(node, Param))
{
if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
Param *param = (Param *) node;
if (param->paramkind != PARAM_EXEC ||
!list_member_int(context->safe_param_ids, param->paramid))
{
if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
}
return false; /* nothing to recurse to */
}
/*