From 060234426851cb8f815fb873ab5aaf33b3830143 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Fri, 3 Apr 2020 18:26:29 +0200 Subject: [PATCH 2/5] rework add_partial_path_precheck too --- src/backend/optimizer/path/joinpath.c | 12 ++++++++--- src/backend/optimizer/util/pathnode.c | 31 ++++++++++++--------------- src/include/optimizer/pathnode.h | 3 ++- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/backend/optimizer/path/joinpath.c b/src/backend/optimizer/path/joinpath.c index db54a6ba2e..1d0c3e8027 100644 --- a/src/backend/optimizer/path/joinpath.c +++ b/src/backend/optimizer/path/joinpath.c @@ -515,7 +515,9 @@ try_partial_nestloop_path(PlannerInfo *root, */ initial_cost_nestloop(root, &workspace, jointype, outer_path, inner_path, extra); - if (!add_partial_path_precheck(joinrel, workspace.total_cost, pathkeys)) + if (!add_partial_path_precheck(joinrel, + workspace.startup_cost, workspace.total_cost, + pathkeys)) return; /* @@ -693,7 +695,9 @@ try_partial_mergejoin_path(PlannerInfo *root, outersortkeys, innersortkeys, extra); - if (!add_partial_path_precheck(joinrel, workspace.total_cost, pathkeys)) + if (!add_partial_path_precheck(joinrel, + workspace.startup_cost, workspace.total_cost, + pathkeys)) return; /* Might be good enough to be worth trying, so let's try it. */ @@ -817,7 +821,9 @@ try_partial_hashjoin_path(PlannerInfo *root, */ initial_cost_hashjoin(root, &workspace, jointype, hashclauses, outer_path, inner_path, extra, parallel_hash); - if (!add_partial_path_precheck(joinrel, workspace.total_cost, NIL)) + if (!add_partial_path_precheck(joinrel, + workspace.startup_cost, workspace.total_cost, + NIL)) return; /* Might be good enough to be worth trying, so let's try it. */ diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index b570bfd3be..7211fc35fd 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -854,15 +854,14 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path) * add_partial_path_precheck * Check whether a proposed new partial path could possibly get accepted. * - * Unlike add_path_precheck, we can ignore startup cost and parameterization, - * since they don't matter for partial paths (see add_partial_path). But - * we do want to make sure we don't add a partial path if there's already - * a complete path that dominates it, since in that case the proposed path - * is surely a loser. + * Unlike add_path_precheck, we can ignore parameterization, since it doesn't + * matter for partial paths (see add_partial_path). But we do want to make + * sure we don't add a partial path if there's already a complete path that + * dominates it, since in that case the proposed path is surely a loser. */ bool -add_partial_path_precheck(RelOptInfo *parent_rel, Cost total_cost, - List *pathkeys) +add_partial_path_precheck(RelOptInfo *parent_rel, Cost startup_cost, + Cost total_cost, List *pathkeys) { ListCell *p1; @@ -885,11 +884,14 @@ add_partial_path_precheck(RelOptInfo *parent_rel, Cost total_cost, keyscmp = compare_pathkeys(pathkeys, old_path->pathkeys); if (keyscmp != PATHKEYS_DIFFERENT) { - if (total_cost > old_path->total_cost * STD_FUZZ_FACTOR && - keyscmp != PATHKEYS_BETTER1) + if ((startup_cost > old_path->startup_cost * STD_FUZZ_FACTOR) && + (total_cost > old_path->total_cost * STD_FUZZ_FACTOR) && + (keyscmp != PATHKEYS_BETTER1)) return false; - if (old_path->total_cost > total_cost * STD_FUZZ_FACTOR && - keyscmp != PATHKEYS_BETTER2) + + if ((old_path->startup_cost > startup_cost * STD_FUZZ_FACTOR) && + (old_path->total_cost > total_cost * STD_FUZZ_FACTOR) && + (keyscmp != PATHKEYS_BETTER2)) return true; } } @@ -899,13 +901,8 @@ add_partial_path_precheck(RelOptInfo *parent_rel, Cost total_cost, * clearly good enough that it might replace one. Compare it to * non-parallel plans. If it loses even before accounting for the cost of * the Gather node, we should definitely reject it. - * - * Note that we pass the total_cost to add_path_precheck twice. This is - * because it's never advantageous to consider the startup cost of a - * partial path; the resulting plans, if run in parallel, will be run to - * completion. */ - if (!add_path_precheck(parent_rel, total_cost, total_cost, pathkeys, + if (!add_path_precheck(parent_rel, startup_cost, total_cost, pathkeys, NULL)) return false; diff --git a/src/include/optimizer/pathnode.h b/src/include/optimizer/pathnode.h index e450fe112a..e73c5637cc 100644 --- a/src/include/optimizer/pathnode.h +++ b/src/include/optimizer/pathnode.h @@ -32,7 +32,8 @@ extern bool add_path_precheck(RelOptInfo *parent_rel, List *pathkeys, Relids required_outer); extern void add_partial_path(RelOptInfo *parent_rel, Path *new_path); extern bool add_partial_path_precheck(RelOptInfo *parent_rel, - Cost total_cost, List *pathkeys); + Cost startup_cost, Cost total_cost, + List *pathkeys); extern Path *create_seqscan_path(PlannerInfo *root, RelOptInfo *rel, Relids required_outer, int parallel_workers); -- 2.21.1