From b12fd41c695b43c76b0a9a4d19ba43b05536440c Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 9 Jun 2016 12:40:23 -0400 Subject: [PATCH] Don't generate parallel paths for rels with parallel-restricted outputs. Such paths are unsafe. To make it cheaper to detect when this case applies, track whether a relation's default PathTarget contains any non-Vars. In most cases, the answer will be no, which enables us to determine cheaply that the target list for a proposed path is parallel-safe. However, subquery pull-up can create cases that require us to inspect the target list more carefully. Amit Kapila, reviewed by me. --- src/backend/nodes/outfuncs.c | 1 + src/backend/optimizer/path/allpaths.c | 10 ++++++++++ src/backend/optimizer/util/placeholder.c | 2 ++ src/backend/optimizer/util/relnode.c | 10 +++++++--- src/include/nodes/relation.h | 2 ++ 5 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index c7b4153c03..2796e5353a 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2083,6 +2083,7 @@ _outRelOptInfo(StringInfo str, const RelOptInfo *node) WRITE_BOOL_FIELD(consider_param_startup); WRITE_BOOL_FIELD(consider_parallel); WRITE_NODE_FIELD(reltarget); + WRITE_BOOL_FIELD(reltarget_has_non_vars); WRITE_NODE_FIELD(pathlist); WRITE_NODE_FIELD(ppilist); WRITE_NODE_FIELD(partial_pathlist); diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index cf6a9a9f82..6deb2cf0c9 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -608,6 +608,15 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, if (has_parallel_hazard((Node *) rel->baserestrictinfo, false)) return; + /* + * If the relation's outputs are not parallel-safe, we must give up. + * In the common case where the relation only outputs Vars, this check is + * very cheap; otherwise, we have to do more work. + */ + if (rel->reltarget_has_non_vars && + has_parallel_hazard((Node *) rel->reltarget->exprs, false)) + return; + /* We have a winner. */ rel->consider_parallel = true; } @@ -971,6 +980,7 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel, adjust_appendrel_attrs(root, (Node *) rel->reltarget->exprs, appinfo); + childrel->reltarget_has_non_vars = rel->reltarget_has_non_vars; /* * We have to make child entries in the EquivalenceClass data diff --git a/src/backend/optimizer/util/placeholder.c b/src/backend/optimizer/util/placeholder.c index b210914b85..5b85a4ddad 100644 --- a/src/backend/optimizer/util/placeholder.c +++ b/src/backend/optimizer/util/placeholder.c @@ -393,6 +393,7 @@ add_placeholders_to_base_rels(PlannerInfo *root) rel->reltarget->exprs = lappend(rel->reltarget->exprs, copyObject(phinfo->ph_var)); + rel->reltarget_has_non_vars = true; /* reltarget's cost and width fields will be updated later */ } } @@ -427,6 +428,7 @@ add_placeholders_to_joinrel(PlannerInfo *root, RelOptInfo *joinrel, /* Yup, add it to the output */ joinrel->reltarget->exprs = lappend(joinrel->reltarget->exprs, phinfo->ph_var); + joinrel->reltarget_has_non_vars = true; joinrel->reltarget->width += phinfo->ph_width; /* diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index e2ebf47929..2def06dd92 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -109,6 +109,7 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptKind reloptkind) rel->consider_parallel = false; /* might get changed later */ rel->rel_parallel_workers = -1; /* set up in GetRelationInfo */ rel->reltarget = create_empty_pathtarget(); + rel->reltarget_has_non_vars = false; rel->pathlist = NIL; rel->ppilist = NIL; rel->partial_pathlist = NIL; @@ -396,6 +397,7 @@ build_join_rel(PlannerInfo *root, joinrel->consider_param_startup = false; joinrel->consider_parallel = false; joinrel->reltarget = create_empty_pathtarget(); + joinrel->reltarget_has_non_vars = false; joinrel->pathlist = NIL; joinrel->ppilist = NIL; joinrel->partial_pathlist = NIL; @@ -506,8 +508,8 @@ build_join_rel(PlannerInfo *root, * Set the consider_parallel flag if this joinrel could potentially be * scanned within a parallel worker. If this flag is false for either * inner_rel or outer_rel, then it must be false for the joinrel also. - * Even if both are true, there might be parallel-restricted quals at our - * level. + * Even if both are true, there might be parallel-restricted expressions + * in the targetlist or quals. * * Note that if there are more than two rels in this relation, they could * be divided between inner_rel and outer_rel in any arbitrary way. We @@ -517,7 +519,9 @@ build_join_rel(PlannerInfo *root, * here. */ if (inner_rel->consider_parallel && outer_rel->consider_parallel && - !has_parallel_hazard((Node *) restrictlist, false)) + !has_parallel_hazard((Node *) restrictlist, false) && + !(joinrel->reltarget_has_non_vars && + has_parallel_hazard((Node *) joinrel->reltarget->exprs, false))) joinrel->consider_parallel = true; /* diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h index 88669f3876..b2b204693f 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -490,6 +490,8 @@ typedef struct RelOptInfo /* default result targetlist for Paths scanning this relation */ struct PathTarget *reltarget; /* list of Vars/Exprs, cost, width */ + bool reltarget_has_non_vars; /* true if any expression in + * PathTarget is a non-Var */ /* materialization information */ List *pathlist; /* Path structures */ -- 2.40.0