Don't generate parallel paths for rels with parallel-restricted outputs.

robertmhaas · robertmhaas · commit b12fd41c695b · 2016-06-09T12:43:36.000-04:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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 */

Original file line number	Diff line number	Diff line change
`@@ -393,6 +393,7 @@ add_placeholders_to_base_rels(PlannerInfo *root)`
`393`	`393`
`394`	`394`	`rel->reltarget->exprs = lappend(rel->reltarget->exprs,`
`395`	`395`	`copyObject(phinfo->ph_var));`
	`396`	`+ rel->reltarget_has_non_vars = true;`
`396`	`397`	`/* reltarget's cost and width fields will be updated later */`
`397`	`398`	`}`
`398`	`399`	`}`
`@@ -427,6 +428,7 @@ add_placeholders_to_joinrel(PlannerInfo root, RelOptInfo joinrel,`
`427`	`428`	`/* Yup, add it to the output */`
`428`	`429`	`joinrel->reltarget->exprs = lappend(joinrel->reltarget->exprs,`
`429`	`430`	`phinfo->ph_var);`
	`431`	`+ joinrel->reltarget_has_non_vars = true;`
`430`	`432`	`joinrel->reltarget->width += phinfo->ph_width;`
`431`	`433`
`432`	`434`	`/*`