public inbox for [email protected]
help / color / mirror / Atom feedFrom: James Coleman <[email protected]>
To: pgsql-hackers <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: Parallelize correlated subqueries that execute within each worker
Date: Fri, 7 May 2021 12:30:02 -0400
Message-ID: <CAAaqYe_x1u3V4uPiv=dJ=k2EJ7txhdq6yexJDkYZ1x1pu0QwcQ@mail.gmail.com> (raw)
In a bug report back in November [1] a subthread explored why parallel
query is excluded any time we have "Plan nodes which reference a
correlated SubPlan". Amit's understanding was that the reasoning had
to do with inability to easily pass (potentially variable length)
Param values between workers.
However a decent-sized subset of this kind of query doesn't actually
require that we communicate between workers. If the Subplan executes
per-tuple within the worker then there's no reason I can see why it
needs to be marked parallel unsafe. Amit concurred but noted that
identifying that subset of plans is the difficult part (as is usually
the case!)
At the time I'd started work on an approach to handle this case and
hoped to "post about it in a new thread later this week." That didn't
happen, but here we are now, and I finally have this patch cleaned up
enough to share.
The basic idea is that we need to track (both on nodes and relations)
not only whether that node or rel is parallel safe but also whether
it's parallel safe assuming params are rechecked in the using context.
That allows us to delay making a final decision until we have
sufficient context to conclude that a given usage of a Param is
actually parallel safe or unsafe.
The first patch in this series was previously posted in the thread
"Consider parallel for lateral subqueries with limit" [2] and is
required as a precursor for various test cases to work here.
The second patch implements the core of the series. It results in
parallel query being possible for subplans that execute entirely
within the context of a parallel worker for cases where that subplan
is in the target, a LATERAL JOIN, or the WHERE and ORDER BY clauses.
The final patch notes several places where we set e.g.
rel->consider_parallel but setting the corresponding new value
rel->consider_parallel_recheckng_params wasn't yet necessary. It shows
opportunity either for further improvement or concluding certain cases
can't benefit and should be left unchanged.
James
1: https://www.postgresql.org/message-id/CAAaqYe_vihKjc%2B8LuQa49EHW4%2BKfefb3wHqPYFnCuUqozo%2BLFg%40ma...
2: https://www.postgresql.org/message-id/flat/CAAaqYe_HEkmLwf_1iEHxXwQOWiRyiFd%3DuOu6kwj3sWYdVd1-zA%40m...
Attachments:
[application/octet-stream] v1-0003-Other-places-to-consider-for-completeness.patch (5.1K, ../CAAaqYe_x1u3V4uPiv=dJ=k2EJ7txhdq6yexJDkYZ1x1pu0QwcQ@mail.gmail.com/2-v1-0003-Other-places-to-consider-for-completeness.patch)
download | inline diff:
From ac93b6513dfd3fd066cbbe51eced2bc00146f0c0 Mon Sep 17 00:00:00 2001
From: jcoleman <[email protected]>
Date: Fri, 7 May 2021 15:37:22 +0000
Subject: [PATCH v1 3/3] Other places to consider for completeness
---
src/backend/optimizer/path/allpaths.c | 4 ++++
src/backend/optimizer/plan/planmain.c | 2 +-
src/backend/optimizer/plan/planner.c | 3 +++
src/backend/optimizer/plan/subselect.c | 3 +++
src/backend/optimizer/prep/prepunion.c | 2 ++
5 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index ae744da135..5696a54b18 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1142,6 +1142,8 @@ set_append_rel_size(PlannerInfo *root, RelOptInfo *rel,
*/
if (!childrel->consider_parallel)
rel->consider_parallel = false;
+ if (!childrel->consider_parallel_rechecking_params)
+ rel->consider_parallel_rechecking_params = false;
/*
* Accumulate size information from each live child.
@@ -1263,6 +1265,8 @@ set_append_rel_pathlist(PlannerInfo *root, RelOptInfo *rel,
*/
if (!rel->consider_parallel)
childrel->consider_parallel = false;
+ if (!childrel->consider_parallel_rechecking_params)
+ rel->consider_parallel_rechecking_params = false;
/*
* Compute the child's access paths.
diff --git a/src/backend/optimizer/plan/planmain.c b/src/backend/optimizer/plan/planmain.c
index bdbce2b87d..5135f9ee97 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, NULL);
+ is_parallel_safe(root, parse->jointree->quals, &final_rel->consider_parallel_rechecking_params);
/*
* 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 28b8f7d8b5..fb35cd78d1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4088,6 +4088,7 @@ create_window_paths(PlannerInfo *root,
if (input_rel->consider_parallel && output_target_parallel_safe &&
is_parallel_safe(root, (Node *) activeWindows, NULL))
window_rel->consider_parallel = true;
+ /* consider_parallel_rechecking_params */
/*
* If the input rel belongs to a single FDW, so does the window rel.
@@ -4292,6 +4293,7 @@ create_distinct_paths(PlannerInfo *root,
* expressions are parallel-safe.
*/
distinct_rel->consider_parallel = input_rel->consider_parallel;
+ distinct_rel->consider_parallel_rechecking_params = input_rel->consider_parallel_rechecking_params;
/*
* If the input rel belongs to a single FDW, so does the distinct_rel.
@@ -4493,6 +4495,7 @@ create_ordered_paths(PlannerInfo *root,
*/
if (input_rel->consider_parallel && target_parallel_safe)
ordered_rel->consider_parallel = true;
+ /* consider_parallel_rechecking_params */
/*
* If the input rel belongs to a single FDW, so does the ordered_rel.
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index fd1cfe6c73..0c6fbf16a2 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -1022,6 +1022,7 @@ SS_process_ctes(PlannerInfo *root)
* parallel-safe.
*/
splan->parallel_safe = false;
+ splan->parallel_safe_ignoring_params = false;
splan->setParam = NIL;
splan->parParam = NIL;
splan->args = NIL;
@@ -2178,6 +2179,7 @@ SS_charge_for_initplans(PlannerInfo *root, RelOptInfo *final_rel)
path->startup_cost += initplan_cost;
path->total_cost += initplan_cost;
path->parallel_safe = false;
+ path->parallel_safe_ignoring_params = false;
}
/*
@@ -2186,6 +2188,7 @@ SS_charge_for_initplans(PlannerInfo *root, RelOptInfo *final_rel)
*/
final_rel->partial_pathlist = NIL;
final_rel->consider_parallel = false;
+ final_rel->consider_parallel_rechecking_params = false;
/* We needn't do set_cheapest() here, caller will do it */
}
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index a57bc1c2a8..c535131144 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -273,6 +273,7 @@ recurse_set_operations(Node *setOp, PlannerInfo *root,
*/
final_rel = fetch_upper_rel(subroot, UPPERREL_FINAL, NULL);
rel->consider_parallel = final_rel->consider_parallel;
+ rel->consider_parallel_rechecking_params = final_rel->consider_parallel_rechecking_params;
/*
* For the moment, we consider only a single Path for the subquery.
@@ -617,6 +618,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
result_rel = fetch_upper_rel(root, UPPERREL_SETOP, relids);
result_rel->reltarget = create_pathtarget(root, tlist);
result_rel->consider_parallel = consider_parallel;
+ /* consider_parallel_rechecking_params */
/*
* Append the child results together.
--
2.20.1
[application/octet-stream] v1-0002-Parallel-query-support-for-basic-correlated-subqu.patch (75.4K, ../CAAaqYe_x1u3V4uPiv=dJ=k2EJ7txhdq6yexJDkYZ1x1pu0QwcQ@mail.gmail.com/3-v1-0002-Parallel-query-support-for-basic-correlated-subqu.patch)
download | inline diff:
From e3df76aa8823ff05d311fd3aa4505a7f7d58b015 Mon Sep 17 00:00:00 2001
From: jcoleman <[email protected]>
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%';
<listitem>
<para>
- Plan nodes which reference a correlated <literal>SubPlan</literal>.
+ Plan nodes which reference a correlated <literal>SubPlan</literal> where
+ the result is shared between workers.
</para>
</listitem>
</itemizedlist>
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
[application/octet-stream] v1-0001-Allow-parallel-LATERAL-subqueries-with-LIMIT-OFFS.patch (3.5K, ../CAAaqYe_x1u3V4uPiv=dJ=k2EJ7txhdq6yexJDkYZ1x1pu0QwcQ@mail.gmail.com/4-v1-0001-Allow-parallel-LATERAL-subqueries-with-LIMIT-OFFS.patch)
download | inline diff:
From 4f51cf83caaffcbe2500898d7d939c885c0c352c Mon Sep 17 00:00:00 2001
From: jcoleman <[email protected]>
Date: Mon, 30 Nov 2020 11:36:35 -0500
Subject: [PATCH v1 1/3] Allow parallel LATERAL subqueries with LIMIT/OFFSET
The code that determined whether or not a rel should be considered for
parallel query excluded subqueries with LIMIT/OFFSET. That's correct in
the general case: as the comment notes that'd mean we have to guarantee
ordering (and claims it's not worth checking that) for results to be
consistent across workers. However there's a simpler case that hasn't
been considered: LATERAL subqueries with LIMIT/OFFSET don't fall under
the same reasoning since they're executed (when not converted to a JOIN)
per tuple anyway, so consistency of results across workers isn't a
factor.
---
src/backend/optimizer/path/allpaths.c | 4 +++-
src/test/regress/expected/select_parallel.out | 15 +++++++++++++++
src/test/regress/sql/select_parallel.sql | 6 ++++++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 353454b183..45e37590d8 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -682,11 +682,13 @@ set_rel_consider_parallel(PlannerInfo *root, RelOptInfo *rel,
* inconsistent results at the top-level. (In some cases, where
* the result is ordered, we could relax this restriction. But it
* doesn't currently seem worth expending extra effort to do so.)
+ * LATERAL is an exception: LIMIT/OFFSET is safe to execute within
+ * workers since the sub-select is executed per tuple
*/
{
Query *subquery = castNode(Query, rte->subquery);
- if (limit_needed(subquery))
+ if (!rte->lateral && limit_needed(subquery))
return;
}
break;
diff --git a/src/test/regress/expected/select_parallel.out b/src/test/regress/expected/select_parallel.out
index 05ebcb284a..224a1da2af 100644
--- a/src/test/regress/expected/select_parallel.out
+++ b/src/test/regress/expected/select_parallel.out
@@ -1040,6 +1040,21 @@ explain (costs off)
Filter: (stringu1 ~~ '%AAAA'::text)
(11 rows)
+-- ...unless it's LATERAL
+savepoint settings;
+set parallel_tuple_cost=0;
+explain (costs off) select t.unique1 from tenk1 t
+join lateral (select t.unique1 from tenk1 offset 0) l on true;
+ QUERY PLAN
+---------------------------------------------------------------------
+ Gather
+ Workers Planned: 4
+ -> Nested Loop
+ -> Parallel Index Only Scan using tenk1_unique1 on tenk1 t
+ -> Index Only Scan using tenk1_hundred on tenk1
+(5 rows)
+
+rollback to savepoint settings;
-- to increase the parallel query test coverage
SAVEPOINT settings;
SET LOCAL force_parallel_mode = 1;
diff --git a/src/test/regress/sql/select_parallel.sql b/src/test/regress/sql/select_parallel.sql
index d31e290ec2..f656764fa9 100644
--- a/src/test/regress/sql/select_parallel.sql
+++ b/src/test/regress/sql/select_parallel.sql
@@ -390,6 +390,12 @@ explain (costs off, verbose)
explain (costs off)
select * from tenk1 a where two in
(select two from tenk1 b where stringu1 like '%AAAA' limit 3);
+-- ...unless it's LATERAL
+savepoint settings;
+set parallel_tuple_cost=0;
+explain (costs off) select t.unique1 from tenk1 t
+join lateral (select t.unique1 from tenk1 offset 0) l on true;
+rollback to savepoint settings;
-- to increase the parallel query test coverage
SAVEPOINT settings;
--
2.20.1
view thread (25+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected]
Subject: Re: Parallelize correlated subqueries that execute within each worker
In-Reply-To: <CAAaqYe_x1u3V4uPiv=dJ=k2EJ7txhdq6yexJDkYZ1x1pu0QwcQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox