agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
Subject: [PATCH 2/5] rework add_partial_path_precheck too
Date: Fri, 3 Apr 2020 18:26:29 +0200
---
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
--mhm2o3dzmb5lomaa
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename="v54-0003-rework-add_partial_path_precheck-check-costs-first.patch"
view thread (3+ 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]
Subject: Re: [PATCH 2/5] rework add_partial_path_precheck too
In-Reply-To: <no-message-id-194784@localhost>
* 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