From 3303ea1a327b41d3b406d7be7a5ce2901e561066 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 10 Jun 2016 16:20:03 -0400 Subject: [PATCH] Remove reltarget_has_non_vars flag. Commit b12fd41c6 added a "reltarget_has_non_vars" field to RelOptInfo, but failed to maintain it accurately. Since its only purpose was to skip calls to has_parallel_hazard() in the simple case where a rel's targetlist is all Vars, and that call is really pretty cheap in that case anyway, it seems like this is just a case of premature optimization. Let's drop the flag and do the calls unconditionally until it's proven that we need more smarts here. --- src/backend/nodes/outfuncs.c | 1 - src/backend/optimizer/path/allpaths.c | 9 +++------ src/backend/optimizer/util/placeholder.c | 2 -- src/backend/optimizer/util/relnode.c | 5 +---- src/include/nodes/relation.h | 2 -- 5 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 2796e5353ae..c7b4153c030 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -2083,7 +2083,6 @@ _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 ff5e39c1aad..cc8ba61bb0c 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -613,12 +613,10 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, 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. + * Likewise, if the relation's outputs are not parallel-safe, give up. + * (Usually, they're just Vars, but sometimes they're not.) */ - if (rel->reltarget_has_non_vars && - has_parallel_hazard((Node *) rel->reltarget->exprs, false)) + if (has_parallel_hazard((Node *) rel->reltarget->exprs, false)) return; /* We have a winner. */ @@ -984,7 +982,6 @@ 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 5b85a4ddadc..b210914b853 100644 --- a/src/backend/optimizer/util/placeholder.c +++ b/src/backend/optimizer/util/placeholder.c @@ -393,7 +393,6 @@ 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 */ } } @@ -428,7 +427,6 @@ 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 91cd2b506fb..ba185aec1a3 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -109,7 +109,6 @@ 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; @@ -397,7 +396,6 @@ 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; @@ -520,8 +518,7 @@ build_join_rel(PlannerInfo *root, */ if (inner_rel->consider_parallel && outer_rel->consider_parallel && !has_parallel_hazard((Node *) restrictlist, false) && - !(joinrel->reltarget_has_non_vars && - has_parallel_hazard((Node *) joinrel->reltarget->exprs, false))) + !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 7ca1e253b9e..cafd6962298 100644 --- a/src/include/nodes/relation.h +++ b/src/include/nodes/relation.h @@ -490,8 +490,6 @@ 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 */