From e3df76aa8823ff05d311fd3aa4505a7f7d58b015 Mon Sep 17 00:00:00 2001 From: jcoleman Date: Fri, 27 Nov 2020 18:44:30 -0500 Subject: [PATCH v1 2/3] Parallel query support for basic correlated subqueries Not all Params are inherently parallel-unsafe, but we can't know whether they're parallel-safe up-front: we need contextual information for a given path shape. Here we delay the final determination of whether or not a Param is parallel-safe by initially verifying that it is minimally parallel-safe for things that are inherent (e.g., no parallel-unsafe functions or relations involved) and later re-checking that a given usage is contextually safe (e.g., the Param is for correlation that can happen entirely within a parallel worker (as opposed to needing to pass values between workers). --- doc/src/sgml/parallel.sgml | 3 +- src/backend/nodes/copyfuncs.c | 2 + src/backend/nodes/equalfuncs.c | 1 + src/backend/nodes/outfuncs.c | 3 + src/backend/nodes/readfuncs.c | 2 + src/backend/optimizer/path/allpaths.c | 61 +++++-- src/backend/optimizer/path/equivclass.c | 4 +- src/backend/optimizer/path/indxpath.c | 20 ++- src/backend/optimizer/path/joinpath.c | 21 ++- src/backend/optimizer/plan/createplan.c | 2 + src/backend/optimizer/plan/planmain.c | 2 +- src/backend/optimizer/plan/planner.c | 155 +++++++++++++++--- src/backend/optimizer/plan/subselect.c | 21 ++- src/backend/optimizer/prep/prepunion.c | 1 + src/backend/optimizer/util/clauses.c | 113 ++++++++++--- src/backend/optimizer/util/pathnode.c | 36 +++- src/backend/optimizer/util/relnode.c | 30 +++- src/backend/utils/misc/guc.c | 11 ++ src/include/nodes/pathnodes.h | 5 +- src/include/nodes/plannodes.h | 1 + src/include/nodes/primnodes.h | 1 + src/include/optimizer/clauses.h | 4 +- .../regress/expected/incremental_sort.out | 28 ++-- src/test/regress/expected/select_parallel.out | 137 ++++++++++++++++ src/test/regress/expected/sysviews.out | 3 +- src/test/regress/sql/select_parallel.sql | 37 +++++ 26 files changed, 604 insertions(+), 100 deletions(-) diff --git a/doc/src/sgml/parallel.sgml b/doc/src/sgml/parallel.sgml index 479e24a1dc..25a689f690 100644 --- a/doc/src/sgml/parallel.sgml +++ b/doc/src/sgml/parallel.sgml @@ -517,7 +517,8 @@ EXPLAIN SELECT * FROM pgbench_accounts WHERE filler LIKE '%x%'; - Plan nodes which reference a correlated SubPlan. + Plan nodes which reference a correlated SubPlan where + the result is shared between workers. diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 632cc31a04..240895ac83 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -121,6 +121,7 @@ CopyPlanFields(const Plan *from, Plan *newnode) COPY_SCALAR_FIELD(parallel_aware); COPY_SCALAR_FIELD(parallel_safe); COPY_SCALAR_FIELD(async_capable); + COPY_SCALAR_FIELD(parallel_safe_ignoring_params); COPY_SCALAR_FIELD(plan_node_id); COPY_NODE_FIELD(targetlist); COPY_NODE_FIELD(qual); @@ -1777,6 +1778,7 @@ _copySubPlan(const SubPlan *from) COPY_SCALAR_FIELD(useHashTable); COPY_SCALAR_FIELD(unknownEqFalse); COPY_SCALAR_FIELD(parallel_safe); + COPY_SCALAR_FIELD(parallel_safe_ignoring_params); COPY_NODE_FIELD(setParam); COPY_NODE_FIELD(parParam); COPY_NODE_FIELD(args); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index a410a29a17..011befabf2 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -459,6 +459,7 @@ _equalSubPlan(const SubPlan *a, const SubPlan *b) COMPARE_SCALAR_FIELD(useHashTable); COMPARE_SCALAR_FIELD(unknownEqFalse); COMPARE_SCALAR_FIELD(parallel_safe); + COMPARE_SCALAR_FIELD(parallel_safe_ignoring_params); COMPARE_NODE_FIELD(setParam); COMPARE_NODE_FIELD(parParam); COMPARE_NODE_FIELD(args); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index c723f6d635..84156d6465 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -334,6 +334,7 @@ _outPlanInfo(StringInfo str, const Plan *node) WRITE_BOOL_FIELD(parallel_aware); WRITE_BOOL_FIELD(parallel_safe); WRITE_BOOL_FIELD(async_capable); + WRITE_BOOL_FIELD(parallel_safe_ignoring_params); WRITE_INT_FIELD(plan_node_id); WRITE_NODE_FIELD(targetlist); WRITE_NODE_FIELD(qual); @@ -1372,6 +1373,7 @@ _outSubPlan(StringInfo str, const SubPlan *node) WRITE_BOOL_FIELD(useHashTable); WRITE_BOOL_FIELD(unknownEqFalse); WRITE_BOOL_FIELD(parallel_safe); + WRITE_BOOL_FIELD(parallel_safe_ignoring_params); WRITE_NODE_FIELD(setParam); WRITE_NODE_FIELD(parParam); WRITE_NODE_FIELD(args); @@ -1770,6 +1772,7 @@ _outPathInfo(StringInfo str, const Path *node) outBitmapset(str, NULL); WRITE_BOOL_FIELD(parallel_aware); WRITE_BOOL_FIELD(parallel_safe); + WRITE_BOOL_FIELD(parallel_safe_ignoring_params); WRITE_INT_FIELD(parallel_workers); WRITE_FLOAT_FIELD(rows, "%.0f"); WRITE_FLOAT_FIELD(startup_cost, "%.2f"); diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 3746668f52..2b22978616 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1620,6 +1620,7 @@ ReadCommonPlan(Plan *local_node) READ_BOOL_FIELD(parallel_aware); READ_BOOL_FIELD(parallel_safe); READ_BOOL_FIELD(async_capable); + READ_BOOL_FIELD(parallel_safe_ignoring_params); READ_INT_FIELD(plan_node_id); READ_NODE_FIELD(targetlist); READ_NODE_FIELD(qual); @@ -2613,6 +2614,7 @@ _readSubPlan(void) READ_BOOL_FIELD(useHashTable); READ_BOOL_FIELD(unknownEqFalse); READ_BOOL_FIELD(parallel_safe); + READ_BOOL_FIELD(parallel_safe_ignoring_params); READ_NODE_FIELD(setParam); READ_NODE_FIELD(parParam); READ_NODE_FIELD(args); diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 45e37590d8..ae744da135 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -556,7 +556,8 @@ set_rel_pathlist(PlannerInfo *root, RelOptInfo *rel, * (see grouping_planner). */ if (rel->reloptkind == RELOPT_BASEREL && - bms_membership(root->all_baserels) != BMS_SINGLETON) + bms_membership(root->all_baserels) != BMS_SINGLETON + && (rel->subplan_params == NIL || rte->rtekind != RTE_SUBQUERY)) generate_useful_gather_paths(root, rel, false); /* Now find the cheapest of the paths for this rel */ @@ -592,6 +593,9 @@ static void set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte) { + bool parallel_safe; + bool parallel_safe_except_in_params; + /* * The flag has previously been initialized to false, so we can just * return if it becomes clear that we can't safely set it. @@ -632,7 +636,7 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, if (proparallel != PROPARALLEL_SAFE) return; - if (!is_parallel_safe(root, (Node *) rte->tablesample->args)) + if (!is_parallel_safe(root, (Node *) rte->tablesample->args, NULL)) return; } @@ -700,7 +704,7 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, case RTE_FUNCTION: /* Check for parallel-restricted functions. */ - if (!is_parallel_safe(root, (Node *) rte->functions)) + if (!is_parallel_safe(root, (Node *) rte->functions, NULL)) return; break; @@ -710,7 +714,7 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, case RTE_VALUES: /* Check for parallel-restricted functions. */ - if (!is_parallel_safe(root, (Node *) rte->values_lists)) + if (!is_parallel_safe(root, (Node *) rte->values_lists, NULL)) return; break; @@ -747,18 +751,28 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel, * outer join clauses work correctly. It would likely break equivalence * classes, too. */ - if (!is_parallel_safe(root, (Node *) rel->baserestrictinfo)) - return; + parallel_safe = is_parallel_safe(root, (Node *) rel->baserestrictinfo, + ¶llel_safe_except_in_params); /* * Likewise, if the relation's outputs are not parallel-safe, give up. * (Usually, they're just Vars, but sometimes they're not.) */ - if (!is_parallel_safe(root, (Node *) rel->reltarget->exprs)) - return; + if (parallel_safe || parallel_safe_except_in_params) + { + bool target_parallel_safe; + bool target_parallel_safe_ignoring_params = false; + + target_parallel_safe = is_parallel_safe(root, + (Node *) rel->reltarget->exprs, + &target_parallel_safe_ignoring_params); + parallel_safe = parallel_safe && target_parallel_safe; + parallel_safe_except_in_params = parallel_safe_except_in_params + && target_parallel_safe_ignoring_params; + } - /* We have a winner. */ - rel->consider_parallel = true; + rel->consider_parallel = parallel_safe; + rel->consider_parallel_rechecking_params = parallel_safe_except_in_params; } /* @@ -2277,9 +2291,21 @@ set_subquery_pathlist(PlannerInfo *root, RelOptInfo *rel, pathkeys, required_outer)); } + /* + * XXX: As far as I can tell, the only time partial paths exist here + * is when we're going to execute multiple partial parths in parallel + * under a gather node (instead of executing paths serially under + * an append node). That means that the subquery scan path here + * is self-contained at this point -- so by definition it can't be + * reliant on lateral relids, which means we'll never have to consider + * rechecking params here. + */ + Assert(!(rel->consider_parallel_rechecking_params && rel->partial_pathlist && !rel->consider_parallel)); + /* If outer rel allows parallelism, do same for partial paths. */ if (rel->consider_parallel && bms_is_empty(required_outer)) { + /* If consider_parallel is false, there should be no partial paths. */ Assert(sub_final_rel->consider_parallel || sub_final_rel->partial_pathlist == NIL); @@ -2633,7 +2659,7 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows) cheapest_partial_path->rows * cheapest_partial_path->parallel_workers; simple_gather_path = (Path *) create_gather_path(root, rel, cheapest_partial_path, rel->reltarget, - NULL, rowsp); + rel->lateral_relids, rowsp); add_path(rel, simple_gather_path); /* @@ -2650,7 +2676,7 @@ generate_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_rows) rows = subpath->rows * subpath->parallel_workers; path = create_gather_merge_path(root, rel, subpath, rel->reltarget, - subpath->pathkeys, NULL, rowsp); + subpath->pathkeys, rel->lateral_relids, rowsp); add_path(rel, &path->path); } } @@ -2752,11 +2778,15 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r double *rowsp = NULL; List *useful_pathkeys_list = NIL; Path *cheapest_partial_path = NULL; + Relids required_outer = rel->lateral_relids; /* If there are no partial paths, there's nothing to do here. */ if (rel->partial_pathlist == NIL) return; + if (!bms_is_subset(required_outer, rel->relids)) + return; + /* Should we override the rel's rowcount estimate? */ if (override_rows) rowsp = &rows; @@ -2828,7 +2858,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r tmp, rel->reltarget, tmp->pathkeys, - NULL, + required_outer, rowsp); add_path(rel, &path->path); @@ -2862,7 +2892,7 @@ generate_useful_gather_paths(PlannerInfo *root, RelOptInfo *rel, bool override_r tmp, rel->reltarget, tmp->pathkeys, - NULL, + required_outer, rowsp); add_path(rel, &path->path); @@ -3041,7 +3071,8 @@ standard_join_search(PlannerInfo *root, int levels_needed, List *initial_rels) /* * Except for the topmost scan/join rel, consider gathering * partial paths. We'll do the same for the topmost scan/join rel - * once we know the final targetlist (see grouping_planner). + * once we know the final targetlist (see + * apply_scanjoin_target_to_paths). */ if (lev < levels_needed) generate_useful_gather_paths(root, rel, false); diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index 6f1abbe47d..4c176d2d49 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -897,7 +897,7 @@ find_computable_ec_member(PlannerInfo *root, * check this last because it's a rather expensive test. */ if (require_parallel_safe && - !is_parallel_safe(root, (Node *) em->em_expr)) + !is_parallel_safe(root, (Node *) em->em_expr, NULL)) continue; return em; /* found usable expression */ @@ -1012,7 +1012,7 @@ relation_can_be_sorted_early(PlannerInfo *root, RelOptInfo *rel, * check this last because it's a rather expensive test. */ if (require_parallel_safe && - !is_parallel_safe(root, (Node *) em->em_expr)) + !is_parallel_safe(root, (Node *) em->em_expr, NULL)) continue; return true; diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c index 0e4e00eaf0..262c129ff5 100644 --- a/src/backend/optimizer/path/indxpath.c +++ b/src/backend/optimizer/path/indxpath.c @@ -1047,10 +1047,21 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, /* * If appropriate, consider parallel index scan. We don't allow * parallel index scan for bitmap index scans. + * + * XXX: Checking rel->consider_parallel_rechecking_params here resulted + * in some odd behavior on: + * select (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) from tenk1 t; + * where the total cost on the chosen plan went *up* considering + * the extra path. + * + * Current working theory is that this method is about base relation + * scans, and we only want parameterized paths to be parallelized as + * companions to existing parallel plans and so don't really care to + * consider a separate parallel index scan here. */ if (index->amcanparallel && - rel->consider_parallel && outer_relids == NULL && - scantype != ST_BITMAPSCAN) + rel->consider_parallel && outer_relids == NULL && + scantype != ST_BITMAPSCAN) { ipath = create_index_path(root, index, index_clauses, @@ -1100,9 +1111,10 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel, result = lappend(result, ipath); /* If appropriate, consider parallel index scan */ + /* XXX: As above here for rel->consider_parallel_rechecking_params? */ if (index->amcanparallel && - rel->consider_parallel && outer_relids == NULL && - scantype != ST_BITMAPSCAN) + rel->consider_parallel && outer_relids == NULL && + scantype != ST_BITMAPSCAN) { ipath = create_index_path(root, index, index_clauses, diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index 4c30c65564..c6af979714 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -690,6 +690,7 @@ try_partial_nestloop_path(PlannerInfo *root, else outerrelids = outerrel->relids; + /* TODO: recheck parallel safety here? */ if (!bms_is_subset(inner_paramrels, outerrelids)) return; } @@ -1714,16 +1715,24 @@ match_unsorted_outer(PlannerInfo *root, * partial path and the joinrel is parallel-safe. However, we can't * handle JOIN_UNIQUE_OUTER, because the outer path will be partial, and * therefore we won't be able to properly guarantee uniqueness. Nor can - * we handle joins needing lateral rels, since partial paths must not be - * parameterized. Similarly, we can't handle JOIN_FULL and JOIN_RIGHT, - * because they can produce false null extended rows. + * we handle JOIN_FULL and JOIN_RIGHT, because they can produce false null + * extended rows. + * + * Partial paths may only have parameters in limited cases + * where the parameterization is fully satisfied without sharing state + * between workers, so we only allow lateral rels on inputs to the join + * if the resulting join contains no lateral rels, the inner rel's laterals + * are fully satisfied by the outer rel, and the outer rel doesn't depend + * on the inner rel to produce any laterals. */ if (joinrel->consider_parallel && save_jointype != JOIN_UNIQUE_OUTER && save_jointype != JOIN_FULL && save_jointype != JOIN_RIGHT && outerrel->partial_pathlist != NIL && - bms_is_empty(joinrel->lateral_relids)) + bms_is_empty(joinrel->lateral_relids) && + bms_is_subset(innerrel->lateral_relids, outerrel->relids) && + (bms_is_empty(outerrel->lateral_relids) || !bms_is_subset(outerrel->lateral_relids, innerrel->relids))) { if (nestjoinOK) consider_parallel_nestloop(root, joinrel, outerrel, innerrel, @@ -1838,7 +1847,9 @@ consider_parallel_nestloop(PlannerInfo *root, Path *rcpath; /* Can't join to an inner path that is not parallel-safe */ - if (!innerpath->parallel_safe) + /* TODO: recheck parallel safety of params here? */ + if (!innerpath->parallel_safe && + !(innerpath->parallel_safe_ignoring_params)) continue; /* diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index a9aff24831..817ce04596 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -5274,6 +5274,7 @@ copy_generic_path_info(Plan *dest, Path *src) dest->plan_width = src->pathtarget->width; dest->parallel_aware = src->parallel_aware; dest->parallel_safe = src->parallel_safe; + dest->parallel_safe_ignoring_params = src->parallel_safe_ignoring_params; } /* @@ -5291,6 +5292,7 @@ copy_plan_costsize(Plan *dest, Plan *src) dest->parallel_aware = false; /* Assume the inserted node is parallel-safe, if child plan is. */ dest->parallel_safe = src->parallel_safe; + dest->parallel_safe_ignoring_params = src->parallel_safe_ignoring_params; } /* diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c index 273ac0acf7..bdbce2b87d 100644 --- a/src/backend/optimizer/plan/planmain.c +++ b/src/backend/optimizer/plan/planmain.c @@ -119,7 +119,7 @@ query_planner(PlannerInfo *root, if (root->glob->parallelModeOK && force_parallel_mode != FORCE_PARALLEL_OFF) final_rel->consider_parallel = - is_parallel_safe(root, parse->jointree->quals); + is_parallel_safe(root, parse->jointree->quals, NULL); /* * The only path for it is a trivial Result path. We cheat a diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 1868c4eff4..28b8f7d8b5 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -150,6 +150,7 @@ static RelOptInfo *create_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, PathTarget *target, bool target_parallel_safe, + bool target_parallel_safe_ignoring_params, grouping_sets_data *gd); static bool is_degenerate_grouping(PlannerInfo *root); static void create_degenerate_grouping_paths(PlannerInfo *root, @@ -157,6 +158,7 @@ static void create_degenerate_grouping_paths(PlannerInfo *root, RelOptInfo *grouped_rel); static RelOptInfo *make_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel, PathTarget *target, bool target_parallel_safe, + bool target_parallel_safe_ignoring_params, Node *havingQual); static void create_ordinary_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, @@ -231,6 +233,7 @@ static void apply_scanjoin_target_to_paths(PlannerInfo *root, List *scanjoin_targets, List *scanjoin_targets_contain_srfs, bool scanjoin_target_parallel_safe, + bool scanjoin_target_parallel_safe_ignoring_params, bool tlist_same_exprs); static void create_partitionwise_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, @@ -1235,6 +1238,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) List *final_targets; List *final_targets_contain_srfs; bool final_target_parallel_safe; + bool final_target_parallel_safe_ignoring_params = false; RelOptInfo *current_rel; RelOptInfo *final_rel; FinalPathExtraData extra; @@ -1297,7 +1301,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) /* And check whether it's parallel safe */ final_target_parallel_safe = - is_parallel_safe(root, (Node *) final_target->exprs); + is_parallel_safe(root, (Node *) final_target->exprs, &final_target_parallel_safe_ignoring_params); /* The setop result tlist couldn't contain any SRFs */ Assert(!parse->hasTargetSRFs); @@ -1331,14 +1335,17 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) List *sort_input_targets; List *sort_input_targets_contain_srfs; bool sort_input_target_parallel_safe; + bool sort_input_target_parallel_safe_ignoring_params = false; PathTarget *grouping_target; List *grouping_targets; List *grouping_targets_contain_srfs; bool grouping_target_parallel_safe; + bool grouping_target_parallel_safe_ignoring_params = false; PathTarget *scanjoin_target; List *scanjoin_targets; List *scanjoin_targets_contain_srfs; bool scanjoin_target_parallel_safe; + bool scanjoin_target_parallel_safe_ignoring_params = false; bool scanjoin_target_same_exprs; bool have_grouping; WindowFuncLists *wflists = NULL; @@ -1451,7 +1458,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) */ final_target = create_pathtarget(root, root->processed_tlist); final_target_parallel_safe = - is_parallel_safe(root, (Node *) final_target->exprs); + is_parallel_safe(root, (Node *) final_target->exprs, &final_target_parallel_safe_ignoring_params); /* * If ORDER BY was given, consider whether we should use a post-sort @@ -1464,12 +1471,13 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) final_target, &have_postponed_srfs); sort_input_target_parallel_safe = - is_parallel_safe(root, (Node *) sort_input_target->exprs); + is_parallel_safe(root, (Node *) sort_input_target->exprs, &sort_input_target_parallel_safe_ignoring_params); } else { sort_input_target = final_target; sort_input_target_parallel_safe = final_target_parallel_safe; + sort_input_target_parallel_safe_ignoring_params = final_target_parallel_safe_ignoring_params; } /* @@ -1483,12 +1491,13 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) final_target, activeWindows); grouping_target_parallel_safe = - is_parallel_safe(root, (Node *) grouping_target->exprs); + is_parallel_safe(root, (Node *) grouping_target->exprs, &grouping_target_parallel_safe_ignoring_params); } else { grouping_target = sort_input_target; grouping_target_parallel_safe = sort_input_target_parallel_safe; + grouping_target_parallel_safe_ignoring_params = sort_input_target_parallel_safe_ignoring_params; } /* @@ -1502,12 +1511,13 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) { scanjoin_target = make_group_input_target(root, final_target); scanjoin_target_parallel_safe = - is_parallel_safe(root, (Node *) scanjoin_target->exprs); + is_parallel_safe(root, (Node *) scanjoin_target->exprs, &scanjoin_target_parallel_safe_ignoring_params); } else { scanjoin_target = grouping_target; scanjoin_target_parallel_safe = grouping_target_parallel_safe; + scanjoin_target_parallel_safe_ignoring_params = grouping_target_parallel_safe_ignoring_params; } /* @@ -1559,6 +1569,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) apply_scanjoin_target_to_paths(root, current_rel, scanjoin_targets, scanjoin_targets_contain_srfs, scanjoin_target_parallel_safe, + scanjoin_target_parallel_safe_ignoring_params, scanjoin_target_same_exprs); /* @@ -1585,6 +1596,7 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) current_rel, grouping_target, grouping_target_parallel_safe, + grouping_target_parallel_safe_ignoring_params, gset_data); /* Fix things up if grouping_target contains SRFs */ if (parse->hasTargetSRFs) @@ -1658,10 +1670,25 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) * not a SELECT, consider_parallel will be false for every relation in the * query. */ - if (current_rel->consider_parallel && - is_parallel_safe(root, parse->limitOffset) && - is_parallel_safe(root, parse->limitCount)) - final_rel->consider_parallel = true; + if (current_rel->consider_parallel || current_rel->consider_parallel_rechecking_params) + { + bool limit_count_parallel_safe; + bool limit_offset_parallel_safe; + bool limit_count_parallel_safe_ignoring_params = false; + bool limit_offset_parallel_safe_ignoring_params = false; + + limit_count_parallel_safe = is_parallel_safe(root, parse->limitCount, &limit_count_parallel_safe_ignoring_params); + limit_offset_parallel_safe = is_parallel_safe(root, parse->limitOffset, &limit_offset_parallel_safe_ignoring_params); + + if (current_rel->consider_parallel && + limit_count_parallel_safe && + limit_offset_parallel_safe) + final_rel->consider_parallel = true; + if (current_rel->consider_parallel_rechecking_params && + limit_count_parallel_safe_ignoring_params && + limit_offset_parallel_safe_ignoring_params) + final_rel->consider_parallel_rechecking_params = true; + } /* * If the current_rel belongs to a single FDW, so does the final_rel. @@ -1862,8 +1889,8 @@ grouping_planner(PlannerInfo *root, double tuple_fraction) * Generate partial paths for final_rel, too, if outer query levels might * be able to make use of them. */ - if (final_rel->consider_parallel && root->query_level > 1 && - !limit_needed(parse)) + if ((final_rel->consider_parallel || final_rel->consider_parallel_rechecking_params) && + root->query_level > 1 && !limit_needed(parse)) { Assert(!parse->rowMarks && parse->commandType == CMD_SELECT); foreach(lc, current_rel->partial_pathlist) @@ -3275,6 +3302,7 @@ create_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, PathTarget *target, bool target_parallel_safe, + bool target_parallel_safe_ignoring_params, grouping_sets_data *gd) { Query *parse = root->parse; @@ -3290,7 +3318,9 @@ create_grouping_paths(PlannerInfo *root, * aggregation paths. */ grouped_rel = make_grouping_rel(root, input_rel, target, - target_parallel_safe, parse->havingQual); + target_parallel_safe, + target_parallel_safe_ignoring_params, + parse->havingQual); /* * Create either paths for a degenerate grouping or paths for ordinary @@ -3351,6 +3381,7 @@ create_grouping_paths(PlannerInfo *root, extra.flags = flags; extra.target_parallel_safe = target_parallel_safe; + extra.target_parallel_safe_ignoring_params = target_parallel_safe_ignoring_params; extra.havingQual = parse->havingQual; extra.targetList = parse->targetList; extra.partial_costs_set = false; @@ -3386,7 +3417,7 @@ create_grouping_paths(PlannerInfo *root, static RelOptInfo * make_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel, PathTarget *target, bool target_parallel_safe, - Node *havingQual) + bool target_parallel_safe_ignoring_params, Node *havingQual) { RelOptInfo *grouped_rel; @@ -3414,9 +3445,21 @@ make_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel, * can't be parallel-safe, either. Otherwise, it's parallel-safe if the * target list and HAVING quals are parallel-safe. */ - if (input_rel->consider_parallel && target_parallel_safe && - is_parallel_safe(root, (Node *) havingQual)) - grouped_rel->consider_parallel = true; + if ((input_rel->consider_parallel || input_rel->consider_parallel_rechecking_params) + && (target_parallel_safe || target_parallel_safe_ignoring_params)) + { + bool having_qual_parallel_safe; + bool having_qual_parallel_safe_ignoring_params = false; + + having_qual_parallel_safe = is_parallel_safe(root, (Node *) havingQual, + &having_qual_parallel_safe_ignoring_params); + + grouped_rel->consider_parallel = input_rel->consider_parallel && + having_qual_parallel_safe && target_parallel_safe; + grouped_rel->consider_parallel_rechecking_params = + input_rel->consider_parallel_rechecking_params && + having_qual_parallel_safe_ignoring_params && target_parallel_safe_ignoring_params; + } /* * If the input rel belongs to a single FDW, so does the grouped rel. @@ -4043,7 +4086,7 @@ create_window_paths(PlannerInfo *root, * target list and active windows for non-parallel-safe constructs. */ if (input_rel->consider_parallel && output_target_parallel_safe && - is_parallel_safe(root, (Node *) activeWindows)) + is_parallel_safe(root, (Node *) activeWindows, NULL)) window_rel->consider_parallel = true; /* @@ -5602,6 +5645,7 @@ adjust_paths_for_srfs(PlannerInfo *root, RelOptInfo *rel, PathTarget *thistarget = lfirst_node(PathTarget, lc1); bool contains_srfs = (bool) lfirst_int(lc2); + /* TODO: How do we know the new target is parallel safe? */ /* If this level doesn't contain SRFs, do regular projection */ if (contains_srfs) newpath = (Path *) create_set_projection_path(root, @@ -5918,8 +5962,8 @@ plan_create_index_workers(Oid tableOid, Oid indexOid) * safe. */ if (heap->rd_rel->relpersistence == RELPERSISTENCE_TEMP || - !is_parallel_safe(root, (Node *) RelationGetIndexExpressions(index)) || - !is_parallel_safe(root, (Node *) RelationGetIndexPredicate(index))) + !is_parallel_safe(root, (Node *) RelationGetIndexExpressions(index), NULL) || + !is_parallel_safe(root, (Node *) RelationGetIndexPredicate(index), NULL)) { parallel_workers = 0; goto done; @@ -6382,6 +6426,8 @@ create_partial_grouping_paths(PlannerInfo *root, grouped_rel->relids); partially_grouped_rel->consider_parallel = grouped_rel->consider_parallel; + partially_grouped_rel->consider_parallel_rechecking_params = + grouped_rel->consider_parallel_rechecking_params; partially_grouped_rel->reloptkind = grouped_rel->reloptkind; partially_grouped_rel->serverid = grouped_rel->serverid; partially_grouped_rel->userid = grouped_rel->userid; @@ -6845,6 +6891,7 @@ apply_scanjoin_target_to_paths(PlannerInfo *root, List *scanjoin_targets, List *scanjoin_targets_contain_srfs, bool scanjoin_target_parallel_safe, + bool scanjoin_target_parallel_safe_ignoring_params, bool tlist_same_exprs) { bool rel_is_partitioned = IS_PARTITIONED_REL(rel); @@ -6854,6 +6901,11 @@ apply_scanjoin_target_to_paths(PlannerInfo *root, /* This recurses, so be paranoid. */ check_stack_depth(); + /* + * TOOD: when/how do we want to generate gather paths if + * scanjoin_target_parallel_safe_ignoring_params = true + */ + /* * If the rel is partitioned, we want to drop its existing paths and * generate new ones. This function would still be correct if we kept the @@ -6894,6 +6946,64 @@ apply_scanjoin_target_to_paths(PlannerInfo *root, generate_useful_gather_paths(root, rel, false); /* Can't use parallel query above this level. */ + + /* + * There are cases where: + * (rel->consider_parallel && + * !scanjoin_target_parallel_safe && + * scanjoin_target_parallel_safe_ignoring_params) + * is true at this point. See longer commment below. + */ + if (!(rel->consider_parallel_rechecking_params && scanjoin_target_parallel_safe_ignoring_params)) + { + /* + * TODO: if we limit this to this condition, we're pushing off the + * checks as to whether or not a given param usage is safe in the + * context of a given path (in the context of a given rel?). That + * almost certainly means we'd have to add other checks later (maybe + * just on lateral/relids and not parallel safety overall), because + * at the end of grouping_planner() we copy partial paths to the + * final_rel, and while that path may be acceptable in some contexts + * it may not be in all contexts. + * + * OTOH if we're only dependent on PARAM_EXEC params, and we already + * know that subpath->param_info == NULL holds (and that seems like + * it must since we were going to replace the path target anyway... + * though the one caveat is from the original form of this function + * we'd only ever actually assert that for paths not partial paths) + * then if a param shows up in the target why would it not be parallel + * safe. + * + * Adding to the mystery even with the original form of this function + * we still end up with parallel paths where I'd expect this to + * disallow them. For example: + * + * SELECT '' AS six, f1 AS "Correlated Field", f3 AS "Second Field" + * FROM SUBSELECT_TBL upper + * WHERE f3 IN ( + * SELECT upper.f1 + f2 + * FROM SUBSELECT_TBL + * WHERE f2 = CAST(f3 AS integer) + * ); + * + * ends up with the correlated query underneath parallel plan despite + * its target containing a param, and therefore this function marking + * the rel as consider_parallel=false and removing the partial paths. + * + * But the plan as a whole is parallel safe, and so the subplan is also + * parallel safe, which means we can incorporate it into a full parallel + * plan. In other words, this is a parallel safe, but not parallel aware + * subplan (and regular, not parallel, seq scan inside that subplan). + * It's not a partial path; it'a a full path that is executed as a subquery. + * + * Current conclusion: it's fine for subplans, which is the case we're + * currently targeting anyway. And it might even be the only case that + * matters at all. + */ + /* rel->consider_parallel_rechecking_params = false; */ + /* rel->partial_pathlist = NIL; */ + } + rel->consider_parallel_rechecking_params = false; rel->partial_pathlist = NIL; rel->consider_parallel = false; } @@ -7026,6 +7136,7 @@ apply_scanjoin_target_to_paths(PlannerInfo *root, child_scanjoin_targets, scanjoin_targets_contain_srfs, scanjoin_target_parallel_safe, + scanjoin_target_parallel_safe_ignoring_params, tlist_same_exprs); /* Save non-dummy children for Append paths. */ @@ -7043,6 +7154,11 @@ apply_scanjoin_target_to_paths(PlannerInfo *root, * avoid creating multiple Gather nodes within the same plan. We must do * this after all paths have been generated and before set_cheapest, since * one of the generated paths may turn out to be the cheapest one. + * + * TODO: This is the same problem as earlier in this function: when allowing + * "parallel safe ignoring params" paths here we don't actually know we are + * safe in any possible context just possibly safe in the context of the + * right rel. */ if (rel->consider_parallel && !IS_OTHER_REL(rel)) generate_useful_gather_paths(root, rel, false); @@ -7146,6 +7262,7 @@ create_partitionwise_grouping_paths(PlannerInfo *root, child_grouped_rel = make_grouping_rel(root, child_input_rel, child_target, extra->target_parallel_safe, + extra->target_parallel_safe_ignoring_params, child_extra.havingQual); /* Create grouping paths for this child relation. */ diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c index 0881a208ac..fd1cfe6c73 100644 --- a/src/backend/optimizer/plan/subselect.c +++ b/src/backend/optimizer/plan/subselect.c @@ -342,6 +342,7 @@ build_subplan(PlannerInfo *root, Plan *plan, PlannerInfo *subroot, splan->useHashTable = false; splan->unknownEqFalse = unknownEqFalse; splan->parallel_safe = plan->parallel_safe; + splan->parallel_safe_ignoring_params = plan->parallel_safe_ignoring_params; splan->setParam = NIL; splan->parParam = NIL; splan->args = NIL; @@ -1939,6 +1940,7 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context) { SubLink *sublink = (SubLink *) node; Node *testexpr; + Node *result; /* * First, recursively process the lefthand-side expressions, if any. @@ -1950,12 +1952,29 @@ process_sublinks_mutator(Node *node, process_sublinks_context *context) /* * Now build the SubPlan node and make the expr to return. */ - return make_subplan(context->root, + result = make_subplan(context->root, (Query *) sublink->subselect, sublink->subLinkType, sublink->subLinkId, testexpr, context->isTopQual); + + /* + * If planning determined that a subpath was parallel safe as long + * as required params are provided by each individual worker then we + * can mark the resulting subplan actually parallel safe since we now + * know for certain how that path will be used. + */ + if (IsA(result, SubPlan) && !((SubPlan*)result)->parallel_safe + && ((SubPlan*)result)->parallel_safe_ignoring_params + && enable_parallel_params_recheck) + { + Plan *subplan = planner_subplan_get_plan(context->root, (SubPlan*)result); + ((SubPlan*)result)->parallel_safe = is_parallel_safe(context->root, testexpr, NULL); + subplan->parallel_safe = ((SubPlan*)result)->parallel_safe; + } + + return result; } /* diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c index 037dfaacfd..a57bc1c2a8 100644 --- a/src/backend/optimizer/prep/prepunion.c +++ b/src/backend/optimizer/prep/prepunion.c @@ -409,6 +409,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root, Assert(subpath->param_info == NULL); /* avoid apply_projection_to_path, in case of multiple refs */ + /* TODO: how to we know the target is parallel safe? */ path = (Path *) create_projection_path(root, subpath->parent, subpath, target); lfirst(lc) = path; diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index d9ad4efc5e..4c188b85a4 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -54,6 +54,15 @@ #include "utils/syscache.h" #include "utils/typcache.h" +bool enable_parallel_params_recheck = true; + +typedef struct +{ + PlannerInfo *root; + AggSplit aggsplit; + AggClauseCosts *costs; +} get_agg_clause_costs_context; + typedef struct { ParamListInfo boundParams; @@ -87,6 +96,8 @@ typedef struct { char max_hazard; /* worst proparallel hazard found so far */ char max_interesting; /* worst proparallel hazard of interest */ + char max_hazard_ignoring_params; + bool check_params_independently; List *safe_param_ids; /* PARAM_EXEC Param IDs to treat as safe */ } max_parallel_hazard_context; @@ -620,19 +631,27 @@ max_parallel_hazard(Query *parse) context.max_hazard = PROPARALLEL_SAFE; context.max_interesting = PROPARALLEL_UNSAFE; context.safe_param_ids = NIL; + context.check_params_independently = false; (void) max_parallel_hazard_walker((Node *) parse, &context); return context.max_hazard; } /* * is_parallel_safe - * Detect whether the given expr contains only parallel-safe functions + * Detect whether the given expr contains only parallel safe funcions + * XXX: It does more than functions? + * + * If provided, safe_ignoring_params will be set the result of running the same + * parallel safety checks with the exception that params will be allowed. This + * value is useful since params are not inherently parallel unsafe, but rather + * their usage context (whether or not the worker is able to provide the value) + * determines parallel safety. * * root->glob->maxParallelHazard must previously have been set to the - * result of max_parallel_hazard() on the whole query. + * result of max_parallel_hazard() on the whole query */ bool -is_parallel_safe(PlannerInfo *root, Node *node) +is_parallel_safe(PlannerInfo *root, Node *node, bool *safe_ignoring_params) { max_parallel_hazard_context context; PlannerInfo *proot; @@ -649,8 +668,10 @@ is_parallel_safe(PlannerInfo *root, Node *node) return true; /* Else use max_parallel_hazard's search logic, but stop on RESTRICTED */ context.max_hazard = PROPARALLEL_SAFE; + context.max_hazard_ignoring_params = PROPARALLEL_SAFE; context.max_interesting = PROPARALLEL_RESTRICTED; context.safe_param_ids = NIL; + context.check_params_independently = safe_ignoring_params != NULL; /* * The params that refer to the same or parent query level are considered @@ -668,12 +689,17 @@ is_parallel_safe(PlannerInfo *root, Node *node) } } - return !max_parallel_hazard_walker(node, &context); + (void) max_parallel_hazard_walker(node, &context); + + if (safe_ignoring_params) + *safe_ignoring_params = context.max_hazard_ignoring_params == PROPARALLEL_SAFE; + + return context.max_hazard == PROPARALLEL_SAFE; } /* core logic for all parallel-hazard checks */ static bool -max_parallel_hazard_test(char proparallel, max_parallel_hazard_context *context) +max_parallel_hazard_test(char proparallel, max_parallel_hazard_context *context, bool from_param) { switch (proparallel) { @@ -684,12 +710,16 @@ max_parallel_hazard_test(char proparallel, max_parallel_hazard_context *context) /* increase max_hazard to RESTRICTED */ Assert(context->max_hazard != PROPARALLEL_UNSAFE); context->max_hazard = proparallel; + if (!from_param) + context->max_hazard_ignoring_params = proparallel; /* done if we are not expecting any unsafe functions */ if (context->max_interesting == proparallel) return true; break; case PROPARALLEL_UNSAFE: context->max_hazard = proparallel; + if (!from_param) + context->max_hazard_ignoring_params = proparallel; /* we're always done at the first unsafe construct */ return true; default: @@ -704,7 +734,41 @@ static bool max_parallel_hazard_checker(Oid func_id, void *context) { return max_parallel_hazard_test(func_parallel(func_id), - (max_parallel_hazard_context *) context); + (max_parallel_hazard_context *) context, false); +} + +static bool +max_parallel_hazard_walker_can_short_circuit(max_parallel_hazard_context *context) +{ + if (!context->check_params_independently) + return true; + + switch (context->max_hazard) + { + case PROPARALLEL_SAFE: + /* nothing to see here, move along */ + break; + case PROPARALLEL_RESTRICTED: + if (context->max_interesting == PROPARALLEL_RESTRICTED) + return context->max_hazard_ignoring_params != PROPARALLEL_SAFE; + + /* + * We haven't even met our max interesting yet, so + * we certainly can't short-circuit. + */ + break; + case PROPARALLEL_UNSAFE: + if (context->max_interesting == PROPARALLEL_RESTRICTED) + return context->max_hazard_ignoring_params != PROPARALLEL_SAFE; + else if (context->max_interesting == PROPARALLEL_UNSAFE) + return context->max_hazard_ignoring_params == PROPARALLEL_UNSAFE; + + break; + default: + elog(ERROR, "unrecognized proparallel value \"%c\"", context->max_hazard); + break; + } + return false; } static bool @@ -716,7 +780,7 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) /* Check for hazardous functions in node itself */ if (check_functions_in_node(node, max_parallel_hazard_checker, context)) - return true; + return max_parallel_hazard_walker_can_short_circuit(context); /* * It should be OK to treat MinMaxExpr as parallel-safe, since btree @@ -731,14 +795,14 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) */ if (IsA(node, CoerceToDomain)) { - if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context)) - return true; + if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context, false)) + return max_parallel_hazard_walker_can_short_circuit(context); } else if (IsA(node, NextValueExpr)) { - if (max_parallel_hazard_test(PROPARALLEL_UNSAFE, context)) - return true; + if (max_parallel_hazard_test(PROPARALLEL_UNSAFE, context, false)) + return max_parallel_hazard_walker_can_short_circuit(context); } /* @@ -751,8 +815,8 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) */ else if (IsA(node, WindowFunc)) { - if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context)) - return true; + if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context, false)) + return max_parallel_hazard_walker_can_short_circuit(context); } /* @@ -771,8 +835,8 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) */ else if (IsA(node, SubLink)) { - if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context)) - return true; + if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context, false)) + return max_parallel_hazard_walker_can_short_circuit(context); } /* @@ -787,18 +851,23 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) List *save_safe_param_ids; if (!subplan->parallel_safe && - max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context)) + (!enable_parallel_params_recheck || !subplan->parallel_safe_ignoring_params) && + max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context, false) && + max_parallel_hazard_walker_can_short_circuit(context)) return true; save_safe_param_ids = context->safe_param_ids; context->safe_param_ids = list_concat_copy(context->safe_param_ids, subplan->paramIds); - if (max_parallel_hazard_walker(subplan->testexpr, context)) - return true; /* no need to restore safe_param_ids */ + if (max_parallel_hazard_walker(subplan->testexpr, context) && + max_parallel_hazard_walker_can_short_circuit(context)) + /* no need to restore safe_param_ids */ + return true; + list_free(context->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; + return max_parallel_hazard_walker_can_short_circuit(context); /* don't want to recurse normally, so we're done */ return false; } @@ -820,8 +889,8 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) if (param->paramkind != PARAM_EXEC || !list_member_int(context->safe_param_ids, param->paramid)) { - if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context)) - return true; + if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context, true)) + return max_parallel_hazard_walker_can_short_circuit(context); } return false; /* nothing to recurse to */ } @@ -839,7 +908,7 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) if (query->rowMarks != NULL) { context->max_hazard = PROPARALLEL_UNSAFE; - return true; + return max_parallel_hazard_walker_can_short_circuit(context); } /* Recurse into subselects */ diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index b248b038e0..93956f3828 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -756,10 +756,10 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path) CHECK_FOR_INTERRUPTS(); /* Path to be added must be parallel safe. */ - Assert(new_path->parallel_safe); + Assert(new_path->parallel_safe || new_path->parallel_safe_ignoring_params); /* Relation should be OK for parallelism, too. */ - Assert(parent_rel->consider_parallel); + Assert(parent_rel->consider_parallel || parent_rel->consider_parallel_rechecking_params); /* * As in add_path, throw out any paths which are dominated by the new @@ -938,6 +938,7 @@ create_seqscan_path(PlannerInfo *root, RelOptInfo *rel, required_outer); pathnode->parallel_aware = parallel_workers > 0 ? true : false; pathnode->parallel_safe = rel->consider_parallel; + pathnode->parallel_safe_ignoring_params = rel->consider_parallel_rechecking_params; pathnode->parallel_workers = parallel_workers; pathnode->pathkeys = NIL; /* seqscan has unordered result */ @@ -1016,6 +1017,7 @@ create_index_path(PlannerInfo *root, required_outer); pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = rel->consider_parallel; + pathnode->path.parallel_safe_ignoring_params = rel->consider_parallel_rechecking_params; pathnode->path.parallel_workers = 0; pathnode->path.pathkeys = pathkeys; @@ -1868,7 +1870,7 @@ create_gather_merge_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath, Cost input_startup_cost = 0; Cost input_total_cost = 0; - Assert(subpath->parallel_safe); + Assert(subpath->parallel_safe || subpath->parallel_safe_ignoring_params); Assert(pathkeys); pathnode->path.pathtype = T_GatherMerge; @@ -1956,7 +1958,7 @@ create_gather_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath, { GatherPath *pathnode = makeNode(GatherPath); - Assert(subpath->parallel_safe); + Assert(subpath->parallel_safe || subpath->parallel_safe_ignoring_params); pathnode->path.pathtype = T_Gather; pathnode->path.parent = rel; @@ -2003,6 +2005,8 @@ create_subqueryscan_path(PlannerInfo *root, RelOptInfo *rel, Path *subpath, pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = rel->consider_parallel && subpath->parallel_safe; + pathnode->path.parallel_safe_ignoring_params = rel->consider_parallel_rechecking_params && + subpath->parallel_safe_ignoring_params; pathnode->path.parallel_workers = subpath->parallel_workers; pathnode->path.pathkeys = pathkeys; pathnode->subpath = subpath; @@ -2420,6 +2424,8 @@ create_nestloop_path(PlannerInfo *root, NestPath *pathnode = makeNode(NestPath); Relids inner_req_outer = PATH_REQ_OUTER(inner_path); + /* TODO: Assert lateral relids subset safety? */ + /* * If the inner path is parameterized by the outer, we must drop any * restrict_clauses that are due to be moved into the inner path. We have @@ -2460,6 +2466,8 @@ create_nestloop_path(PlannerInfo *root, pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = joinrel->consider_parallel && outer_path->parallel_safe && inner_path->parallel_safe; + pathnode->path.parallel_safe_ignoring_params = joinrel->consider_parallel_rechecking_params && + outer_path->parallel_safe_ignoring_params && inner_path->parallel_safe_ignoring_params; /* This is a foolish way to estimate parallel_workers, but for now... */ pathnode->path.parallel_workers = outer_path->parallel_workers; pathnode->path.pathkeys = pathkeys; @@ -2633,6 +2641,8 @@ create_projection_path(PlannerInfo *root, { ProjectionPath *pathnode = makeNode(ProjectionPath); PathTarget *oldtarget = subpath->pathtarget; + bool target_parallel_safe; + bool target_parallel_safe_ignoring_params = false; pathnode->path.pathtype = T_Result; pathnode->path.parent = rel; @@ -2640,9 +2650,12 @@ create_projection_path(PlannerInfo *root, /* For now, assume we are above any joins, so no parameterization */ pathnode->path.param_info = NULL; pathnode->path.parallel_aware = false; + target_parallel_safe = is_parallel_safe(root, (Node *) target->exprs, + &target_parallel_safe_ignoring_params); pathnode->path.parallel_safe = rel->consider_parallel && - subpath->parallel_safe && - is_parallel_safe(root, (Node *) target->exprs); + subpath->parallel_safe && target_parallel_safe; + pathnode->path.parallel_safe_ignoring_params = rel->consider_parallel_rechecking_params && + subpath->parallel_safe_ignoring_params && target_parallel_safe_ignoring_params; pathnode->path.parallel_workers = subpath->parallel_workers; /* Projection does not change the sort order */ pathnode->path.pathkeys = subpath->pathkeys; @@ -2749,7 +2762,7 @@ apply_projection_to_path(PlannerInfo *root, * parallel-safe in the target expressions, then we can't. */ if ((IsA(path, GatherPath) || IsA(path, GatherMergePath)) && - is_parallel_safe(root, (Node *) target->exprs)) + is_parallel_safe(root, (Node *) target->exprs, NULL)) { /* * We always use create_projection_path here, even if the subpath is @@ -2783,7 +2796,7 @@ apply_projection_to_path(PlannerInfo *root, } } else if (path->parallel_safe && - !is_parallel_safe(root, (Node *) target->exprs)) + !is_parallel_safe(root, (Node *) target->exprs, NULL)) { /* * We're inserting a parallel-restricted target list into a path @@ -2791,6 +2804,7 @@ apply_projection_to_path(PlannerInfo *root, * safe. */ path->parallel_safe = false; + path->parallel_safe_ignoring_params = false; /* TODO */ } return path; @@ -2823,7 +2837,7 @@ create_set_projection_path(PlannerInfo *root, pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = rel->consider_parallel && subpath->parallel_safe && - is_parallel_safe(root, (Node *) target->exprs); + is_parallel_safe(root, (Node *) target->exprs, NULL); pathnode->path.parallel_workers = subpath->parallel_workers; /* Projection does not change the sort order XXX? */ pathnode->path.pathkeys = subpath->pathkeys; @@ -3100,6 +3114,8 @@ create_agg_path(PlannerInfo *root, pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = rel->consider_parallel && subpath->parallel_safe; + pathnode->path.parallel_safe_ignoring_params = rel->consider_parallel_rechecking_params && + subpath->parallel_safe_ignoring_params; pathnode->path.parallel_workers = subpath->parallel_workers; if (aggstrategy == AGG_SORTED) pathnode->path.pathkeys = subpath->pathkeys; /* preserves order */ @@ -3721,6 +3737,8 @@ create_limit_path(PlannerInfo *root, RelOptInfo *rel, pathnode->path.parallel_aware = false; pathnode->path.parallel_safe = rel->consider_parallel && subpath->parallel_safe; + pathnode->path.parallel_safe_ignoring_params = rel->consider_parallel_rechecking_params && + subpath->parallel_safe_ignoring_params; pathnode->path.parallel_workers = subpath->parallel_workers; pathnode->path.rows = subpath->rows; pathnode->path.startup_cost = subpath->startup_cost; diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index e105a4d5f1..d026db8759 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -213,6 +213,7 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent) rel->consider_startup = (root->tuple_fraction > 0); rel->consider_param_startup = false; /* might get changed later */ rel->consider_parallel = false; /* might get changed later */ + rel->consider_parallel_rechecking_params = false; /* might get changed later */ rel->reltarget = create_empty_pathtarget(); rel->pathlist = NIL; rel->ppilist = NIL; @@ -616,6 +617,7 @@ build_join_rel(PlannerInfo *root, joinrel->consider_startup = (root->tuple_fraction > 0); joinrel->consider_param_startup = false; joinrel->consider_parallel = false; + joinrel->consider_parallel_rechecking_params = false; joinrel->reltarget = create_empty_pathtarget(); joinrel->pathlist = NIL; joinrel->ppilist = NIL; @@ -741,10 +743,27 @@ build_join_rel(PlannerInfo *root, * take; therefore, we should make the same decision here however we get * here. */ - if (inner_rel->consider_parallel && outer_rel->consider_parallel && - is_parallel_safe(root, (Node *) restrictlist) && - is_parallel_safe(root, (Node *) joinrel->reltarget->exprs)) - joinrel->consider_parallel = true; + if ((inner_rel->consider_parallel || inner_rel->consider_parallel_rechecking_params) + && (outer_rel->consider_parallel || outer_rel->consider_parallel_rechecking_params)) + { + bool restrictlist_parallel_safe; + bool restrictlist_parallel_safe_ignoring_params = false; + bool target_parallel_safe; + bool target_parallel_safe_ignoring_params = false; + + restrictlist_parallel_safe = is_parallel_safe(root, (Node *) restrictlist, &restrictlist_parallel_safe_ignoring_params); + target_parallel_safe = is_parallel_safe(root, (Node *) joinrel->reltarget->exprs, &target_parallel_safe_ignoring_params); + + if (inner_rel->consider_parallel && outer_rel->consider_parallel + && restrictlist_parallel_safe && target_parallel_safe) + joinrel->consider_parallel = true; + + if (inner_rel->consider_parallel_rechecking_params + && outer_rel->consider_parallel_rechecking_params + && restrictlist_parallel_safe_ignoring_params + && target_parallel_safe_ignoring_params) + joinrel->consider_parallel_rechecking_params = true; + } /* Add the joinrel to the PlannerInfo. */ add_join_rel(root, joinrel); @@ -803,6 +822,7 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel, joinrel->consider_startup = (root->tuple_fraction > 0); joinrel->consider_param_startup = false; joinrel->consider_parallel = false; + joinrel->consider_parallel_rechecking_params = false; joinrel->reltarget = create_empty_pathtarget(); joinrel->pathlist = NIL; joinrel->ppilist = NIL; @@ -889,6 +909,7 @@ build_child_join_rel(PlannerInfo *root, RelOptInfo *outer_rel, /* Child joinrel is parallel safe if parent is parallel safe. */ joinrel->consider_parallel = parent_joinrel->consider_parallel; + joinrel->consider_parallel_rechecking_params = parent_joinrel->consider_parallel_rechecking_params; /* Set estimates of the child-joinrel's size. */ set_joinrel_size_estimates(root, joinrel, outer_rel, inner_rel, @@ -1233,6 +1254,7 @@ fetch_upper_rel(PlannerInfo *root, UpperRelationKind kind, Relids relids) upperrel->consider_startup = (root->tuple_fraction > 0); upperrel->consider_param_startup = false; upperrel->consider_parallel = false; /* might get changed later */ + upperrel->consider_parallel_rechecking_params = false; /* might get changed later */ upperrel->reltarget = create_empty_pathtarget(); upperrel->pathlist = NIL; upperrel->cheapest_startup_path = NULL; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index b130874bdc..1fbab55e2c 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -58,6 +58,7 @@ #include "libpq/libpq.h" #include "libpq/pqformat.h" #include "miscadmin.h" +#include "optimizer/clauses.h" #include "optimizer/cost.h" #include "optimizer/geqo.h" #include "optimizer/optimizer.h" @@ -955,6 +956,16 @@ static const unit_conversion time_unit_conversion_table[] = static struct config_bool ConfigureNamesBool[] = { + { + {"enable_parallel_params_recheck", PGC_USERSET, QUERY_TUNING_METHOD, + gettext_noop("Enables the planner's rechecking of parallel safety in the presence of PARAM_EXEC params (for correlated subqueries)."), + NULL, + GUC_EXPLAIN + }, + &enable_parallel_params_recheck, + true, + NULL, NULL, NULL + }, { {"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD, gettext_noop("Enables the planner's use of sequential-scan plans."), diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h index a65bda7e3c..2ecbb016a2 100644 --- a/src/include/nodes/pathnodes.h +++ b/src/include/nodes/pathnodes.h @@ -682,6 +682,7 @@ typedef struct RelOptInfo bool consider_startup; /* keep cheap-startup-cost paths? */ bool consider_param_startup; /* ditto, for parameterized paths? */ bool consider_parallel; /* consider parallel paths? */ + bool consider_parallel_rechecking_params; /* consider parallel paths? */ /* default result targetlist for Paths scanning this relation */ struct PathTarget *reltarget; /* list of Vars/Exprs, cost, width */ @@ -1178,6 +1179,7 @@ typedef struct Path bool parallel_aware; /* engage parallel-aware logic? */ bool parallel_safe; /* OK to use as part of parallel plan? */ + bool parallel_safe_ignoring_params; /* OK to use as part of parallel plan if worker context provides params? */ int parallel_workers; /* desired # of workers; 0 = not parallel */ /* estimated size/costs for path (see costsize.c for more info) */ @@ -2464,7 +2466,7 @@ typedef struct MinMaxAggInfo * for conflicting purposes. * * In addition, PARAM_EXEC slots are assigned for Params representing outputs - * from subplans (values that are setParam items for those subplans). These + * from subplans (values that are setParam items for those subplans). [TODO: is this true, or only for init plans?] These * IDs need not be tracked via PlannerParamItems, since we do not need any * duplicate-elimination nor later processing of the represented expressions. * Instead, we just record the assignment of the slot number by appending to @@ -2583,6 +2585,7 @@ typedef struct /* Data which may differ across partitions. */ bool target_parallel_safe; + bool target_parallel_safe_ignoring_params; Node *havingQual; List *targetList; PartitionwiseAggregateType patype; diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index d671328dfd..b36def462b 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -128,6 +128,7 @@ typedef struct Plan */ bool parallel_aware; /* engage parallel-aware logic? */ bool parallel_safe; /* OK to use as part of parallel plan? */ + bool parallel_safe_ignoring_params; /* OK to use as part of parallel plan if worker context provides params? */ /* * information needed for asynchronous execution diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 9ae851d847..279a8e106b 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -754,6 +754,7 @@ typedef struct SubPlan * spec result is UNKNOWN; this allows much * simpler handling of null values */ bool parallel_safe; /* is the subplan parallel-safe? */ + bool parallel_safe_ignoring_params; /* is the subplan parallel-safe when params are provided by the worker context? */ /* Note: parallel_safe does not consider contents of testexpr or args */ /* Information for passing params into and out of the subselect: */ /* setParam and parParam are lists of integers (param IDs) */ diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index 0673887a85..df01be2c61 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -16,6 +16,8 @@ #include "nodes/pathnodes.h" +extern PGDLLIMPORT bool enable_parallel_params_recheck; + typedef struct { int numWindowFuncs; /* total number of WindowFuncs found */ @@ -33,7 +35,7 @@ extern double expression_returns_set_rows(PlannerInfo *root, Node *clause); extern bool contain_subplans(Node *clause); extern char max_parallel_hazard(Query *parse); -extern bool is_parallel_safe(PlannerInfo *root, Node *node); +extern bool is_parallel_safe(PlannerInfo *root, Node *node, bool *safe_ignoring_params); extern bool contain_nonstrict_functions(Node *clause); extern bool contain_exec_param(Node *clause, List *param_ids); extern bool contain_leaked_vars(Node *clause); diff --git a/src/test/regress/expected/incremental_sort.out b/src/test/regress/expected/incremental_sort.out index 545e301e48..8f9ca05e60 100644 --- a/src/test/regress/expected/incremental_sort.out +++ b/src/test/regress/expected/incremental_sort.out @@ -1614,16 +1614,16 @@ from tenk1 t, generate_series(1, 1000); QUERY PLAN --------------------------------------------------------------------------------- Unique - -> Sort - Sort Key: t.unique1, ((SubPlan 1)) - -> Gather - Workers Planned: 2 + -> Gather Merge + Workers Planned: 2 + -> Sort + Sort Key: t.unique1, ((SubPlan 1)) -> Nested Loop -> Parallel Index Only Scan using tenk1_unique1 on tenk1 t -> Function Scan on generate_series - SubPlan 1 - -> Index Only Scan using tenk1_unique1 on tenk1 - Index Cond: (unique1 = t.unique1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on tenk1 + Index Cond: (unique1 = t.unique1) (11 rows) explain (costs off) select @@ -1633,16 +1633,16 @@ from tenk1 t, generate_series(1, 1000) order by 1, 2; QUERY PLAN --------------------------------------------------------------------------- - Sort - Sort Key: t.unique1, ((SubPlan 1)) - -> Gather - Workers Planned: 2 + Gather Merge + Workers Planned: 2 + -> Sort + Sort Key: t.unique1, ((SubPlan 1)) -> Nested Loop -> Parallel Index Only Scan using tenk1_unique1 on tenk1 t -> Function Scan on generate_series - SubPlan 1 - -> Index Only Scan using tenk1_unique1 on tenk1 - Index Cond: (unique1 = t.unique1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on tenk1 + Index Cond: (unique1 = t.unique1) (10 rows) -- Parallel sort but with expression not available until the upper rel. diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out index 224a1da2af..e6f2453a1f 100644 --- a/src/test/regress/expected/select_parallel.out +++ b/src/test/regress/expected/select_parallel.out @@ -311,6 +311,131 @@ select count(*) from tenk1 where (two, four) not in 10000 (1 row) +-- test parallel plans for queries containing correlated subplans +-- where the subplan only needs params available from the current +-- worker's scan. +explain (costs off, verbose) select + (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) + from tenk1 t, generate_series(1, 10); + QUERY PLAN +---------------------------------------------------------------------------- + Gather + Output: ((SubPlan 1)) + Workers Planned: 4 + -> Nested Loop + Output: (SubPlan 1) + -> Parallel Index Only Scan using tenk1_unique1 on public.tenk1 t + Output: t.unique1 + -> Function Scan on pg_catalog.generate_series + Output: generate_series.generate_series + Function Call: generate_series(1, 10) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on public.tenk1 + Output: t.unique1 + Index Cond: (tenk1.unique1 = t.unique1) +(14 rows) + +explain (costs off, verbose) select + (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) + from tenk1 t; + QUERY PLAN +---------------------------------------------------------------------- + Gather + Output: ((SubPlan 1)) + Workers Planned: 4 + -> Parallel Index Only Scan using tenk1_unique1 on public.tenk1 t + Output: (SubPlan 1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on public.tenk1 + Output: t.unique1 + Index Cond: (tenk1.unique1 = t.unique1) +(9 rows) + +explain (analyze, costs off, summary off, verbose, timing off) select + (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) + from tenk1 t + limit 1; + QUERY PLAN +---------------------------------------------------------------------------------------------------- + Limit (actual rows=1 loops=1) + Output: ((SubPlan 1)) + -> Gather (actual rows=1 loops=1) + Output: ((SubPlan 1)) + Workers Planned: 4 + Workers Launched: 4 + -> Parallel Index Only Scan using tenk1_unique1 on public.tenk1 t (actual rows=1 loops=5) + Output: (SubPlan 1) + Heap Fetches: 0 + Worker 0: actual rows=1 loops=1 + Worker 1: actual rows=1 loops=1 + Worker 2: actual rows=1 loops=1 + Worker 3: actual rows=1 loops=1 + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on public.tenk1 (actual rows=1 loops=5) + Output: t.unique1 + Index Cond: (tenk1.unique1 = t.unique1) + Heap Fetches: 0 + Worker 0: actual rows=1 loops=1 + Worker 1: actual rows=1 loops=1 + Worker 2: actual rows=1 loops=1 + Worker 3: actual rows=1 loops=1 +(22 rows) + +explain (costs off, verbose) select t.unique1 + from tenk1 t + where t.unique1 = (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1); + QUERY PLAN +---------------------------------------------------------------------- + Gather + Output: t.unique1 + Workers Planned: 4 + -> Parallel Index Only Scan using tenk1_unique1 on public.tenk1 t + Output: t.unique1 + Filter: (t.unique1 = (SubPlan 1)) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on public.tenk1 + Output: t.unique1 + Index Cond: (tenk1.unique1 = t.unique1) +(10 rows) + +explain (costs off, verbose) select * + from tenk1 t + order by (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1); + QUERY PLAN +---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- + Gather Merge + Output: t.unique1, t.unique2, t.two, t.four, t.ten, t.twenty, t.hundred, t.thousand, t.twothousand, t.fivethous, t.tenthous, t.odd, t.even, t.stringu1, t.stringu2, t.string4, ((SubPlan 1)) + Workers Planned: 4 + -> Sort + Output: t.unique1, t.unique2, t.two, t.four, t.ten, t.twenty, t.hundred, t.thousand, t.twothousand, t.fivethous, t.tenthous, t.odd, t.even, t.stringu1, t.stringu2, t.string4, ((SubPlan 1)) + Sort Key: ((SubPlan 1)) + -> Parallel Seq Scan on public.tenk1 t + Output: t.unique1, t.unique2, t.two, t.four, t.ten, t.twenty, t.hundred, t.thousand, t.twothousand, t.fivethous, t.tenthous, t.odd, t.even, t.stringu1, t.stringu2, t.string4, (SubPlan 1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on public.tenk1 + Output: t.unique1 + Index Cond: (tenk1.unique1 = t.unique1) +(12 rows) + +-- test subplan in join/lateral join +explain (costs off, verbose, timing off) select t.unique1, l.* + from tenk1 t + join lateral ( + select (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1 offset 0) + ) l on true; + QUERY PLAN +---------------------------------------------------------------------- + Gather + Output: t.unique1, ((SubPlan 1)) + Workers Planned: 4 + -> Parallel Index Only Scan using tenk1_unique1 on public.tenk1 t + Output: t.unique1, (SubPlan 1) + SubPlan 1 + -> Index Only Scan using tenk1_unique1 on public.tenk1 + Output: t.unique1 + Index Cond: (tenk1.unique1 = t.unique1) +(9 rows) + -- this is not parallel-safe due to use of random() within SubLink's testexpr: explain (costs off) select * from tenk1 where (unique1 + random())::integer not in @@ -1169,6 +1294,18 @@ EXECUTE pstmt('1', make_some_array(1,2)); DEALLOCATE pstmt; -- test interaction between subquery and partial_paths +-- this plan changes to using a non-parallel index only +-- scan on tenk1_unique1 (the parallel version of the subquery scan +-- is cheaper, but only by ~30, and cost comparison treats them as equal +-- since the costs are so large) because set_rel_consider_parallel +-- called from make_one_rel sees the subplan as parallel safe now +-- (in context it now knows the params are actually parallel safe). +-- Because of that the non-parallel index path is now parallel_safe=true, +-- therefore it wins the COSTS_EQUAL comparison in add_path. +-- Perhaps any is_parallel_safe calls made for the purpose of determining +-- consider_parallel should disable that behavior? It's not clear which is +-- correct. +set enable_parallel_params_recheck = off; CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1; EXPLAIN (COSTS OFF) SELECT 1 FROM tenk1_vw_sec diff --git a/src/test/regress/expected/sysviews.out b/src/test/regress/expected/sysviews.out index 0bb558d93c..22fee51455 100644 --- a/src/test/regress/expected/sysviews.out +++ b/src/test/regress/expected/sysviews.out @@ -108,6 +108,7 @@ select name, setting from pg_settings where name like 'enable%'; enable_nestloop | on enable_parallel_append | on enable_parallel_hash | on + enable_parallel_params_recheck | on enable_partition_pruning | on enable_partitionwise_aggregate | off enable_partitionwise_join | off @@ -115,7 +116,7 @@ select name, setting from pg_settings where name like 'enable%'; enable_seqscan | on enable_sort | on enable_tidscan | on -(20 rows) +(21 rows) -- Test that the pg_timezone_names and pg_timezone_abbrevs views are -- more-or-less working. We can't test their contents in any great detail diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql index f656764fa9..55a3bd9025 100644 --- a/src/test/regress/sql/select_parallel.sql +++ b/src/test/regress/sql/select_parallel.sql @@ -111,6 +111,31 @@ explain (costs off) (select hundred, thousand from tenk2 where thousand > 100); select count(*) from tenk1 where (two, four) not in (select hundred, thousand from tenk2 where thousand > 100); +-- test parallel plans for queries containing correlated subplans +-- where the subplan only needs params available from the current +-- worker's scan. +explain (costs off, verbose) select + (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) + from tenk1 t, generate_series(1, 10); +explain (costs off, verbose) select + (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) + from tenk1 t; +explain (analyze, costs off, summary off, verbose, timing off) select + (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1) + from tenk1 t + limit 1; +explain (costs off, verbose) select t.unique1 + from tenk1 t + where t.unique1 = (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1); +explain (costs off, verbose) select * + from tenk1 t + order by (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1); +-- test subplan in join/lateral join +explain (costs off, verbose, timing off) select t.unique1, l.* + from tenk1 t + join lateral ( + select (select t.unique1 from tenk1 where tenk1.unique1 = t.unique1 offset 0) + ) l on true; -- this is not parallel-safe due to use of random() within SubLink's testexpr: explain (costs off) select * from tenk1 where (unique1 + random())::integer not in @@ -452,6 +477,18 @@ EXECUTE pstmt('1', make_some_array(1,2)); DEALLOCATE pstmt; -- test interaction between subquery and partial_paths +-- this plan changes to using a non-parallel index only +-- scan on tenk1_unique1 (the parallel version of the subquery scan +-- is cheaper, but only by ~30, and cost comparison treats them as equal +-- since the costs are so large) because set_rel_consider_parallel +-- called from make_one_rel sees the subplan as parallel safe now +-- (in context it now knows the params are actually parallel safe). +-- Because of that the non-parallel index path is now parallel_safe=true, +-- therefore it wins the COSTS_EQUAL comparison in add_path. +-- Perhaps any is_parallel_safe calls made for the purpose of determining +-- consider_parallel should disable that behavior? It's not clear which is +-- correct. +set enable_parallel_params_recheck = off; CREATE VIEW tenk1_vw_sec WITH (security_barrier) AS SELECT * FROM tenk1; EXPLAIN (COSTS OFF) SELECT 1 FROM tenk1_vw_sec -- 2.20.1