From 269df4be255b3b117e1ff27ba9b340594aacaf2c Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Sun, 28 Jul 2019 15:55:54 +0200 Subject: [PATCH v33 1/3] Consider low startup cost when adding partial path 45be99f8cd5d606086e0a458c9c72910ba8a613d added `add_partial_path` with the comment: > Neither do we need to consider startup costs: > parallelism is only used for plans that will be run to completion. > Therefore, this routine is much simpler than add_path: it needs to > consider only pathkeys and total cost. I'm not entirely sure if that is still true or not--I can't easily come up with a scenario in which it's not, but I also can't come up with an inherent reason why such a scenario cannot exist. Regardless, the in-progress incremental sort patch uncovered a new case where it definitely no longer holds, and, as a result a higher cost plan ends up being chosen because a low startup cost partial path is ignored in favor of a lower total cost partial path and a limit is a applied on top of that which would normal favor the lower startup cost plan. --- src/backend/optimizer/util/pathnode.c | 47 ++++++++++----------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c index d9ce516211..3e836e6e1c 100644 --- a/src/backend/optimizer/util/pathnode.c +++ b/src/backend/optimizer/util/pathnode.c @@ -777,41 +777,30 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path) /* Unless pathkeys are incompatible, keep just one of the two paths. */ if (keyscmp != PATHKEYS_DIFFERENT) { - if (new_path->total_cost > old_path->total_cost * STD_FUZZ_FACTOR) - { - /* New path costs more; keep it only if pathkeys are better. */ - if (keyscmp != PATHKEYS_BETTER1) - accept_new = false; - } - else if (old_path->total_cost > new_path->total_cost - * STD_FUZZ_FACTOR) + PathCostComparison costcmp; + + /* + * Do a fuzzy cost comparison with standard fuzziness limit. + */ + costcmp = compare_path_costs_fuzzily(new_path, old_path, + STD_FUZZ_FACTOR); + + if (costcmp == COSTS_BETTER1) { - /* Old path costs more; keep it only if pathkeys are better. */ - if (keyscmp != PATHKEYS_BETTER2) + if (keyscmp == PATHKEYS_BETTER1) remove_old = true; } - else if (keyscmp == PATHKEYS_BETTER1) - { - /* Costs are about the same, new path has better pathkeys. */ - remove_old = true; - } - else if (keyscmp == PATHKEYS_BETTER2) + else if (costcmp == COSTS_BETTER2) { - /* Costs are about the same, old path has better pathkeys. */ - accept_new = false; - } - else if (old_path->total_cost > new_path->total_cost * 1.0000000001) - { - /* Pathkeys are the same, and the old path costs more. */ - remove_old = true; + if (keyscmp == PATHKEYS_BETTER2) + accept_new = false; } - else + else if (costcmp == COSTS_EQUAL) { - /* - * Pathkeys are the same, and new path isn't materially - * cheaper. - */ - accept_new = false; + if (keyscmp == PATHKEYS_BETTER1) + remove_old = true; + else if (keyscmp == PATHKEYS_BETTER2) + accept_new = false; } } base-commit: b9c3de62cbc9c6993ceac0de99985cf051e91c88 -- 2.17.1