From d0ee7837d2e8137390f6e64188ecbd4a1addb11d Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tv@fuzzy.cz>
Date: Thu, 19 Mar 2020 16:55:28 +0100
Subject: [PATCH 4/8] fix

---
 src/backend/commands/explain.c       |  6 ++--
 src/backend/executor/execProcnode.c  | 11 ++++----
 src/backend/optimizer/plan/planner.c | 41 +++++++++++++++++-----------
 src/backend/utils/sort/tuplesort.c   | 10 +++++--
 src/include/nodes/plannodes.h        |  1 -
 5 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index dd4600a214..0256dd42f1 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2767,7 +2767,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 
 		foreach(methodCell, groupInfo->sortMethods)
 		{
-			const	   *sortMethodName = tuplesort_method_name(methodCell->int_value);
+			const char *sortMethodName = tuplesort_method_name(methodCell->int_value);
 
 			methodNames = lappend(methodNames, sortMethodName);
 		}
@@ -2776,7 +2776,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 		if (groupInfo->maxMemorySpaceUsed > 0)
 		{
 			long		avgSpace = groupInfo->totalMemorySpaceUsed / groupInfo->groupCount;
-			const	   *spaceTypeName;
+			const char *spaceTypeName;
 
 			ExplainPropertyInteger("Average Sort Space Used", "kB", avgSpace, es);
 			ExplainPropertyInteger("Maximum Sort Space Used", "kB",
@@ -2787,7 +2787,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 		if (groupInfo->maxDiskSpaceUsed > 0)
 		{
 			long		avgSpace = groupInfo->totalDiskSpaceUsed / groupInfo->groupCount;
-			const	   *spaceTypeName;
+			const char *spaceTypeName;
 
 			ExplainPropertyInteger("Average Sort Space Used", "kB", avgSpace, es);
 			ExplainPropertyInteger("Maximum Sort Space Used", "kB",
diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c
index d15a86a706..5662e7d742 100644
--- a/src/backend/executor/execProcnode.c
+++ b/src/backend/executor/execProcnode.c
@@ -852,12 +852,13 @@ ExecSetTupleBound(int64 tuples_needed, PlanState *child_node)
 	else if (IsA(child_node, IncrementalSortState))
 	{
 		/*
-		 * If it is a Sort node, notify it that it can use bounded sort.
+		 * If it is an IncrementalSort node, notify it that it can use bounded
+		 * sort.
 		 *
-		 * Note: it is the responsibility of nodeSort.c to react properly to
-		 * changes of these parameters.  If we ever redesign this, it'd be a
-		 * good idea to integrate this signaling with the parameter-change
-		 * mechanism.
+		 * Note: it is the responsibility of nodeIncrementalSort.c to react
+		 * properly to changes of these parameters.  If we ever redesign this,
+		 * it'd be a good idea to integrate this signaling with the
+		 * parameter-change mechanism.
 		 */
 		IncrementalSortState *sortState = (IncrementalSortState *) child_node;
 
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index b2f4aaadb5..0e01cf8cb1 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4869,7 +4869,7 @@ create_distinct_paths(PlannerInfo *root,
 	else
 	{
 		Size		hashentrysize = hash_agg_entry_size(
-														0, cheapest_input_path->pathtarget->width, 0);
+			0, cheapest_input_path->pathtarget->width, 0);
 
 		/* Allow hashing only if hashtable is predicted to fit in work_mem */
 		allow_hash = (hashentrysize * numDistinctRows <= work_mem * 1024L);
@@ -4932,6 +4932,9 @@ create_distinct_paths(PlannerInfo *root,
  * target: the output tlist the result Paths must emit
  * limit_tuples: estimated bound on the number of output tuples,
  *		or -1 if no LIMIT or couldn't estimate
+ *
+ * XXX This only looks at sort_pathkeys. I wonder if it needs to look at the
+ * other pathkeys (grouping, ...) like generate_useful_gather_paths.
  */
 static RelOptInfo *
 create_ordered_paths(PlannerInfo *root,
@@ -5002,23 +5005,29 @@ create_ordered_paths(PlannerInfo *root,
 
 				add_path(ordered_rel, sorted_path);
 			}
-			if (enable_incrementalsort && presorted_keys > 0)
-			{
-				/* Also consider incremental sort. */
-				sorted_path = (Path *) create_incremental_sort_path(root,
-																	ordered_rel,
-																	input_path,
-																	root->sort_pathkeys,
-																	presorted_keys,
-																	limit_tuples);
 
-				/* Add projection step if needed */
-				if (sorted_path->pathtarget != target)
-					sorted_path = apply_projection_to_path(root, ordered_rel,
-														   sorted_path, target);
+			/* With incremental sort disabled, don't build those paths. */
+			if (!enable_incrementalsort)
+				continue;
 
-				add_path(ordered_rel, sorted_path);
-			}
+			/* Likewise, if the path can't be used for incremental sort. */
+			if (!presorted_keys)
+				continue;
+
+			/* Also consider incremental sort. */
+			sorted_path = (Path *) create_incremental_sort_path(root,
+																ordered_rel,
+																input_path,
+																root->sort_pathkeys,
+																presorted_keys,
+																limit_tuples);
+
+			/* Add projection step if needed */
+			if (sorted_path->pathtarget != target)
+				sorted_path = apply_projection_to_path(root, ordered_rel,
+													   sorted_path, target);
+
+			add_path(ordered_rel, sorted_path);
 		}
 	}
 
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 583551d197..77c15ebd78 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -256,8 +256,8 @@ struct Tuplesortstate
 	bool		isMaxSpaceDisk; /* true when maxSpace is value for on-disk
 								 * space, false when it's value for in-memory
 								 * space */
-	TupSortStatus maxSpaceStatus;	/* sort status when maxSpace was reached */
-	MemoryContext maincontext;	/* memory context for tuple sort metadata that
+	TupSortStatus	maxSpaceStatus;	/* sort status when maxSpace was reached */
+	MemoryContext	maincontext;	/* memory context for tuple sort metadata that
 								 * persists across multiple batches */
 	MemoryContext sortcontext;	/* memory context holding most sort data */
 	MemoryContext tuplecontext; /* sub-context of sortcontext for tuple data */
@@ -803,6 +803,9 @@ tuplesort_begin_common(int workMem, SortCoordinate coordinate,
 	return state;
 }
 
+/*
+ * XXX Missing comment.
+ */
 static void
 tuplesort_begin_batch(Tuplesortstate *state)
 {
@@ -1288,6 +1291,9 @@ tuplesort_set_bound(Tuplesortstate *state, int64 bound)
 	state->sortKeys->abbrev_full_comparator = NULL;
 }
 
+/*
+ * XXX Missing comment.
+ */
 bool
 tuplesort_used_bound(Tuplesortstate *state)
 {
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index fe4046b64b..136d794219 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -774,7 +774,6 @@ typedef struct Sort
 	bool	   *nullsFirst;		/* NULLS FIRST/LAST directions */
 } Sort;
 
-
 /* ----------------
  *		incremental sort node
  * ----------------
-- 
2.21.1

