cleanup_get_cheapest_group_keys_order.patch

text/plain

Filename: cleanup_get_cheapest_group_keys_order.patch
Type: text/plain
Part: 0
Message: Some clean-up work in get_cheapest_group_keys_order()

Patch

Format: unified
File+
src/backend/nodes/list.c 21 0
src/backend/optimizer/path/pathkeys.c 18 16
src/include/nodes/pg_list.h 1 0
diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c
index 9d8f4fd5c7..b969a52dd6 100644
--- a/src/backend/nodes/list.c
+++ b/src/backend/nodes/list.c
@@ -1584,6 +1584,27 @@ list_copy(const List *oldlist)
 	return newlist;
 }
 
+/*
+ * Return a shallow copy of the specified list containing only the first 'len'
+ * elements.  If oldlist is shorter than 'len' then we copy the entire list.
+ */
+List *
+list_copy_head(const List *oldlist, int len)
+{
+	List	   *newlist;
+
+	len = Min(oldlist->length, len);
+
+	if (len <= 0)
+		return NIL;
+
+	newlist = new_list(oldlist->type, len);
+	memcpy(newlist->elements, oldlist->elements, len * sizeof(ListCell));
+
+	check_list_invariants(newlist);
+	return newlist;
+}
+
 /*
  * Return a shallow copy of the specified list, without the first N elements.
  */
diff --git a/src/backend/optimizer/path/pathkeys.c b/src/backend/optimizer/path/pathkeys.c
index 9775c4a722..849abbaaba 100644
--- a/src/backend/optimizer/path/pathkeys.c
+++ b/src/backend/optimizer/path/pathkeys.c
@@ -17,6 +17,8 @@
  */
 #include "postgres.h"
 
+#include <float.h>
+
 #include "miscadmin.h"
 #include "access/stratnum.h"
 #include "catalog/pg_opfamily.h"
@@ -591,7 +593,7 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
 
 	ListCell   *cell;
 	PathkeyMutatorState mstate;
-	double		cheapest_sort_cost = -1.0;
+	double		cheapest_sort_cost = DBL_MAX;
 
 	int			nFreeKeys;
 	int			nToPermute;
@@ -620,23 +622,23 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
 	nToPermute = 4;
 	if (nFreeKeys > nToPermute)
 	{
-		int			i;
 		PathkeySortCost *costs = palloc(sizeof(PathkeySortCost) * nFreeKeys);
+		PathkeySortCost *cost = costs;
 
-		/* skip the pre-ordered pathkeys */
-		cell = list_nth_cell(*group_pathkeys, n_preordered);
-
-		/* estimate cost for sorting individual pathkeys */
-		for (i = 0; cell != NULL; i++, (cell = lnext(*group_pathkeys, cell)))
+		/*
+		 * Estimate cost for sorting individual pathkeys skipping the
+		 * pre-ordered pathkeys.
+		 */
+		for_each_from(cell, *group_pathkeys, n_preordered)
 		{
-			List	   *to_cost = list_make1(lfirst(cell));
-
-			Assert(i < nFreeKeys);
+			PathKey	   *pathkey = (PathKey *) lfirst(cell);
+			List	   *to_cost = list_make1(pathkey);
 
-			costs[i].pathkey = lfirst(cell);
-			costs[i].cost = cost_sort_estimate(root, to_cost, 0, nrows);
+			cost->pathkey = pathkey;
+			cost->cost = cost_sort_estimate(root, to_cost, 0, nrows);
+			cost++;
 
-			pfree(to_cost);
+			list_free(to_cost);
 		}
 
 		/* sort the pathkeys by sort cost in ascending order */
@@ -646,9 +648,9 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
 		 * Rebuild the list of pathkeys - first the preordered ones, then the
 		 * rest ordered by cost.
 		 */
-		new_group_pathkeys = list_truncate(list_copy(*group_pathkeys), n_preordered);
+		new_group_pathkeys = list_copy_head(*group_pathkeys, n_preordered);
 
-		for (i = 0; i < nFreeKeys; i++)
+		for (int i = 0; i < nFreeKeys; i++)
 			new_group_pathkeys = lappend(new_group_pathkeys, costs[i].pathkey);
 
 		pfree(costs);
@@ -689,7 +691,7 @@ get_cheapest_group_keys_order(PlannerInfo *root, double nrows,
 
 		cost = cost_sort_estimate(root, var_group_pathkeys, n_preordered, nrows);
 
-		if (cost < cheapest_sort_cost || cheapest_sort_cost < 0)
+		if (cost < cheapest_sort_cost)
 		{
 			list_free(new_group_pathkeys);
 			new_group_pathkeys = list_copy(var_group_pathkeys);
diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h
index 66e70263b2..a6e04d04a3 100644
--- a/src/include/nodes/pg_list.h
+++ b/src/include/nodes/pg_list.h
@@ -620,6 +620,7 @@ extern void list_free(List *list);
 extern void list_free_deep(List *list);
 
 extern pg_nodiscard List *list_copy(const List *list);
+extern pg_nodiscard List *list_copy_head(const List *oldlist, int len);
 extern pg_nodiscard List *list_copy_tail(const List *list, int nskip);
 extern pg_nodiscard List *list_copy_deep(const List *oldlist);