incremental-sort-v22-review.diff

commit 7603a0d1bdcf1ebead6f4671c8e2db96436c86ef
Author: Tomas Vondra <tomas@2ndquadrant.com>
Date:   Fri Apr 6 16:10:56 2018 +0200

    review

diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 8d39b5c..2668cf3 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -1564,14 +1564,12 @@ create_gather_merge_plan(PlannerInfo *root, GatherMergePath *best_path)
 
 	/* Now, insert a Sort node if subplan isn't sufficiently ordered */
 	if (!pathkeys_contained_in(pathkeys, best_path->subpath->pathkeys))
-	{
 		subplan = (Plan *) make_sort(subplan, gm_plan->numCols,
 									 0,
 									 gm_plan->sortColIdx,
 									 gm_plan->sortOperators,
 									 gm_plan->collations,
 									 gm_plan->nullsFirst);
-	}
 
 	/* Now insert the subplan under GatherMerge. */
 	gm_plan->plan.lefttree = subplan;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 6a595c3..6ff98b0 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -4817,6 +4817,8 @@ create_distinct_paths(PlannerInfo *root,
  * The only new path we need consider is an explicit sort on the
  * cheapest-total existing path.
  *
+ * XXX This comment needs updating, I guess.
+ *
  * input_rel: contains the source-data Paths
  * target: the output tlist the result Paths must emit
  * limit_tuples: estimated bound on the number of output tuples,
@@ -4856,12 +4858,24 @@ create_ordered_paths(PlannerInfo *root,
 	{
 		Path	   *path = (Path *) lfirst(lc);
 		int			n_useful_pathkeys;
+		bool		is_partially_sorted;
+		bool		is_sorted;
 
 		n_useful_pathkeys = pathkeys_useful_for_ordering(root->sort_pathkeys,
 														 path->pathkeys);
-		if (path == cheapest_input_path || n_useful_pathkeys > 0)
+
+		/*
+		 * The path is consireder partially sorted when it's sorted by
+		 * a prefix of the pathkeys (possibly all of them).
+		 */
+		is_partially_sorted = (n_useful_pathkeys > 0);
+
+		/* It's fully sorted when it's sorted by all requested keys. */
+		is_sorted = (n_useful_pathkeys == list_length(root->sort_pathkeys));
+
+		if (path == cheapest_input_path || is_partially_sorted)
 		{
-			if (n_useful_pathkeys < list_length(root->sort_pathkeys))
+			if (!is_sorted)
 			{
 				/* An explicit sort here can take advantage of LIMIT */
 				path = (Path *) create_sort_path(root,
@@ -6235,17 +6249,28 @@ add_paths_to_grouping_rel(PlannerInfo *root, RelOptInfo *input_rel,
 		{
 			Path	   *path = (Path *) lfirst(lc);
 			int			n_useful_pathkeys;
+			bool		is_partially_sorted;
+			bool		is_sorted;
 
 			n_useful_pathkeys = pathkeys_useful_for_ordering(
 									root->group_pathkeys, path->pathkeys);
-			if (path == cheapest_path || n_useful_pathkeys > 0)
+			/*
+			 * The path is consireder partially sorted when it's sorted by
+			 * a prefix of the pathkeys (possibly all of them).
+			 */
+			is_partially_sorted = (n_useful_pathkeys > 0);
+
+			/* It's fully sorted when it's sorted by all requested keys. */
+			is_sorted = (n_useful_pathkeys == list_length(root->sort_pathkeys));
+
+			if (path == cheapest_path || is_partially_sorted)
 			{
 				/*
 				 * Sort the path if it isn't already sorted.  Sort might
 				 * be needed for cheapest-total or path sorted by prefix
 				 * of required pathkeys.
 				 */
-				if (n_useful_pathkeys < list_length(root->group_pathkeys))
+				if (!is_sorted)
 					path = (Path *) create_sort_path(root,
 													 grouped_rel,
 													 path,
diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c
index 83665e0..f8d105b 100644
--- a/src/backend/utils/sort/tuplesort.c
+++ b/src/backend/utils/sort/tuplesort.c
@@ -661,7 +661,6 @@ static void free_sort_tuple(Tuplesortstate *state, SortTuple *stup);
 static void tuplesort_free(Tuplesortstate *state);
 static void tuplesort_updatemax(Tuplesortstate *state);
 
-
 /*
  * Special versions of qsort just for SortTuple objects.  qsort_tuple() sorts
  * any variant of SortTuples, using the appropriate comparetup function.
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 5c207e7..815c567 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -1532,7 +1532,6 @@ typedef struct IncrementalSortPath
 	int			presortedCols;	/* number of presorted columns */
 } IncrementalSortPath;
 
-
 /*
  * GroupPath represents grouping (of presorted input)
  *