v50-0004-add-fast-path-to-partial-path-consideration.patch

text/x-patch

Filename: v50-0004-add-fast-path-to-partial-path-consideration.patch
Type: text/x-patch
Part: 0
Message: Re: [PATCH] Incremental sort (was: PoC: Partial sort)

Patch

Format: format-patch
Series: patch v50-0004
Subject: add fast path to partial path consideration
File+
src/backend/optimizer/util/pathnode.c 65 18
From 82c72ef60f9e3879a0c3733f38bf1d61843d0824 Mon Sep 17 00:00:00 2001
From: jcoleman <jtc331@gmail.com>
Date: Wed, 1 Apr 2020 01:01:01 +0000
Subject: [PATCH v50 4/6] add fast path to partial path consideration

---
 src/backend/optimizer/util/pathnode.c | 83 +++++++++++++++++++++------
 1 file changed, 65 insertions(+), 18 deletions(-)

diff --git a/src/backend/optimizer/util/pathnode.c b/src/backend/optimizer/util/pathnode.c
index 5e752f64b9..e444aef60a 100644
--- a/src/backend/optimizer/util/pathnode.c
+++ b/src/backend/optimizer/util/pathnode.c
@@ -779,36 +779,83 @@ add_partial_path(RelOptInfo *parent_rel, Path *new_path)
 		 * Unless pathkeys are incompatible, see if one of the paths dominates
 		 * the other (both in startup and total cost). It may happen that one
 		 * path has lower startup cost, the other has lower total cost.
-		 *
-		 * XXX Perhaps we could do this only when incremental sort is enabled,
-		 * and use the simpler version (comparing just total cost) otherwise?
 		 */
 		if (keyscmp != PATHKEYS_DIFFERENT)
 		{
-			PathCostComparison costcmp;
-
 			/*
-			 * Do a fuzzy cost comparison with standard fuzziness limit.
+			 * It's not entirely obvious that we only need to consider startup
+			 * cost when incremental sort is enabled. But doing so saves us ~1%
+			 * of planning time in some worst case scenarios. We have to
+			 * consider startup cost though for incremental sort, because that
+			 * planner option uncovers scenarios where a total higher cost query
+			 * plans over lower cost ones because a lower startup cost but
+			 * higher total cost path is ignored in favor of a higher startup
+			 * cost (but lower total cost plan) before LIMIT optimizations can
+			 * be applied.
 			 */
-			costcmp = compare_path_costs_fuzzily(new_path, old_path,
-												 STD_FUZZ_FACTOR);
-
-			if (costcmp == COSTS_BETTER1)
+			if (enable_incrementalsort)
 			{
-				if (keyscmp == PATHKEYS_BETTER1)
-					remove_old = true;
+				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)
+				{
+					if (keyscmp == PATHKEYS_BETTER1)
+						remove_old = true;
+				}
+				else if (costcmp == COSTS_BETTER2)
+				{
+					if (keyscmp == PATHKEYS_BETTER2)
+						accept_new = false;
+				}
+				else if (costcmp == COSTS_EQUAL)
+				{
+					if (keyscmp == PATHKEYS_BETTER1)
+						remove_old = true;
+					else if (keyscmp == PATHKEYS_BETTER2)
+						accept_new = false;
+				}
 			}
-			else if (costcmp == COSTS_BETTER2)
+			else if (new_path->total_cost > old_path->total_cost * STD_FUZZ_FACTOR)
 			{
-				if (keyscmp == PATHKEYS_BETTER2)
+				/* New path costs more; keep it only if pathkeys are better. */
+				if (keyscmp != PATHKEYS_BETTER1)
 					accept_new = false;
 			}
-			else if (costcmp == COSTS_EQUAL)
+			else if (old_path->total_cost > new_path->total_cost
+					 * STD_FUZZ_FACTOR)
 			{
-				if (keyscmp == PATHKEYS_BETTER1)
+				/* Old path costs more; keep it only if pathkeys are better. */
+				if (keyscmp != PATHKEYS_BETTER2)
 					remove_old = true;
-				else if (keyscmp == PATHKEYS_BETTER2)
-					accept_new = false;
+			}
+			else if (keyscmp == PATHKEYS_BETTER1)
+			{
+				/* Costs are about the same, new path has better pathkeys. */
+				remove_old = true;
+			}
+			else if (keyscmp == PATHKEYS_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;
+			}
+			else
+			{
+				/*
+				 * Pathkeys are the same, and new path isn't materially
+				 * cheaper.
+				 */
+				accept_new = false;
 			}
 		}
 
-- 
2.17.1