v45-0007-explain-fixes.patch

From 28199b4ffe9a59dc3fab29b30af762eed0040c81 Mon Sep 17 00:00:00 2001
From: James Coleman <jtc331@gmail.com>
Date: Sat, 28 Mar 2020 22:35:49 -0400
Subject: [PATCH v45 7/7] explain fixes

---
 src/backend/commands/explain.c             | 70 ++++++++++++----------
 src/backend/executor/nodeIncrementalSort.c | 63 ++++++++++---------
 src/include/nodes/execnodes.h              |  2 +-
 src/include/utils/tuplesort.h              | 11 ++--
 4 files changed, 76 insertions(+), 70 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 39d51848b6..24acde506e 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2713,26 +2713,41 @@ show_sort_info(SortState *sortstate, ExplainState *es)
  */
 static void
 show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
-								 const char *groupLabel, ExplainState *es)
+								 const char *groupLabel, bool indent, ExplainState *es)
 {
 	ListCell   *methodCell;
-	int			methodCount = list_length(groupInfo->sortMethods);
+	List	   *methodNames = NIL;
+
+	/* Generate a list of sort methods used across all groups. */
+	for (int bit = 0; bit < sizeof(Size); ++bit)
+	{
+		if (groupInfo->sortMethods & (1 << bit))
+		{
+			TuplesortMethod sortMethod = (1 << bit);
+			const char *methodName;
+
+			methodName = tuplesort_method_name(sortMethod);
+			methodNames = lappend(methodNames, unconstify(char *, methodName));
+		}
+	}
 
 	if (es->format == EXPLAIN_FORMAT_TEXT)
 	{
-		appendStringInfoSpaces(es->str, es->indent * 2);
-		appendStringInfo(es->str, "%s Groups: %ld (Methods: ", groupLabel,
+		if (indent)
+			appendStringInfoSpaces(es->str, es->indent * 2);
+		appendStringInfo(es->str, "%s Groups: %ld Sort Method", groupLabel,
 						 groupInfo->groupCount);
-		foreach(methodCell, groupInfo->sortMethods)
+		/* plural/singular based on methodNames size */
+		if (list_length(methodNames) > 1)
+			appendStringInfo(es->str, "s: ");
+		else
+			appendStringInfo(es->str, ": ");
+		foreach(methodCell, methodNames)
 		{
-			const char *sortMethodName;
-
-			sortMethodName = tuplesort_method_name(methodCell->int_value);
-			appendStringInfo(es->str, "%s", sortMethodName);
-			if (foreach_current_index(methodCell) < methodCount - 1)
+			appendStringInfo(es->str, "%s", (char *) methodCell->ptr_value);
+			if (foreach_current_index(methodCell) < list_length(methodNames) - 1)
 				appendStringInfo(es->str, ", ");
 		}
-		appendStringInfo(es->str, ")");
 
 		if (groupInfo->maxMemorySpaceUsed > 0)
 		{
@@ -2740,7 +2755,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 			const char *spaceTypeName;
 
 			spaceTypeName = tuplesort_space_type_name(SORT_SPACE_TYPE_MEMORY);
-			appendStringInfo(es->str, " %s: %ldkB (avg), %ldkB (max)",
+			appendStringInfo(es->str, " %s: avg=%ldkB peak=%ldkB",
 							 spaceTypeName, avgSpace,
 							 groupInfo->maxMemorySpaceUsed);
 		}
@@ -2755,7 +2770,7 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 			/* Add a semicolon separator only if memory stats were printed. */
 			if (groupInfo->maxMemorySpaceUsed > 0)
 				appendStringInfo(es->str, ";");
-			appendStringInfo(es->str, " %s: %ldkB (avg), %ldkB (max)",
+			appendStringInfo(es->str, " %s: avg=%ldkB peak=%ldkB",
 							 spaceTypeName, avgSpace,
 							 groupInfo->maxDiskSpaceUsed);
 		}
@@ -2764,7 +2779,6 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 	}
 	else
 	{
-		List	   *methodNames = NIL;
 		StringInfoData groupName;
 
 		initStringInfo(&groupName);
@@ -2772,12 +2786,6 @@ show_incremental_sort_group_info(IncrementalSortGroupInfo *groupInfo,
 		ExplainOpenGroup("Incremental Sort Groups", groupName.data, true, es);
 		ExplainPropertyInteger("Group Count", NULL, groupInfo->groupCount, es);
 
-		foreach(methodCell, groupInfo->sortMethods)
-		{
-			const char *sortMethodName = tuplesort_method_name(methodCell->int_value);
-
-			methodNames = lappend(methodNames, unconstify(char *, sortMethodName));
-		}
 		ExplainPropertyList("Sort Methods Used", methodNames, es);
 
 		if (groupInfo->maxMemorySpaceUsed > 0)
@@ -2834,10 +2842,10 @@ show_incremental_sort_info(IncrementalSortState *incrsortstate,
 	if (!(es->analyze && fullsortGroupInfo->groupCount > 0))
 		return;
 
-	show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", es);
+	show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", true, es);
 	prefixsortGroupInfo = &incrsortstate->incsort_info.prefixsortGroupInfo;
 	if (prefixsortGroupInfo->groupCount > 0)
-		show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", es);
+		show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", true, es);
 
 	if (incrsortstate->shared_info != NULL)
 	{
@@ -2860,20 +2868,18 @@ show_incremental_sort_info(IncrementalSortState *incrsortstate,
 				prefixsortGroupInfo->groupCount == 0)
 				continue;
 
-			if (!opened_group)
-			{
-				ExplainOpenGroup("Workers", "Workers", false, es);
-				opened_group = true;
-			}
+			if (es->workers_state)
+				ExplainOpenWorker(n, es);
 
 			if (fullsortGroupInfo->groupCount > 0)
-				show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort", es);
+				show_incremental_sort_group_info(fullsortGroupInfo, "Full-sort",
+												 es->workers_state == NULL, es);
 			if (prefixsortGroupInfo->groupCount > 0)
-				show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", es);
-		}
+				show_incremental_sort_group_info(prefixsortGroupInfo, "Presorted", true, es);
 
-		if (opened_group)
-			ExplainCloseGroup("Workers", "Workers", false, es);
+			if (es->workers_state)
+				ExplainCloseWorker(n, es);
+		}
 	}
 }
 
diff --git a/src/backend/executor/nodeIncrementalSort.c b/src/backend/executor/nodeIncrementalSort.c
index 9fe93d5979..eb5515beb8 100644
--- a/src/backend/executor/nodeIncrementalSort.c
+++ b/src/backend/executor/nodeIncrementalSort.c
@@ -85,6 +85,27 @@
 #include "utils/lsyscache.h"
 #include "utils/tuplesort.h"
 
+/*
+ * We need to store the instrumentation information in either local node's sort
+ * info or, for a parallel worker process, in the shared info (this avoids
+ * having to additionally memcpy the info from local memory to shared memory
+ * at each instrumentation call). This macro expands to choose the proper sort
+ * state and group info.
+ *
+ * Arguments:
+ * - node: type IncrementalSortState *
+ * - groupName: the token fullsort or prefixsort
+ */
+#define INSTRUMENT_SORT_GROUP(node, groupName) \
+	if (node->shared_info && node->am_worker) \
+	{ \
+		Assert(IsParallelWorker()); \
+		Assert(ParallelWorkerNumber <= node->shared_info->num_workers); \
+		instrumentSortedGroup(&node->shared_info->sinfo[ParallelWorkerNumber].groupName##GroupInfo, node->groupName##_state); \
+	} else { \
+		instrumentSortedGroup(&node->incsort_info.groupName##GroupInfo, node->groupName##_state); \
+	}
+
 /* ----------------------------------------------------------------
  * instrumentSortedGroup
  *
@@ -94,12 +115,10 @@
  * ----------------------------------------------------------------
  */
 static void
-instrumentSortedGroup(PlanState *pstate, IncrementalSortGroupInfo *groupInfo,
+instrumentSortedGroup(IncrementalSortGroupInfo *groupInfo,
 					  Tuplesortstate *sortState)
 {
-	IncrementalSortState *node = castNode(IncrementalSortState, pstate);
 	TuplesortInstrumentation sort_instr;
-
 	groupInfo->groupCount++;
 
 	tuplesort_get_stats(sortState, &sort_instr);
@@ -122,19 +141,7 @@ instrumentSortedGroup(PlanState *pstate, IncrementalSortGroupInfo *groupInfo,
 	}
 
 	/* Track each sort method we've used. */
-	if (!list_member_int(groupInfo->sortMethods, sort_instr.sortMethod))
-		groupInfo->sortMethods = lappend_int(groupInfo->sortMethods,
-											 sort_instr.sortMethod);
-
-	/* Record shared stats if we're a parallel worker. */
-	if (node->shared_info && node->am_worker)
-	{
-		Assert(IsParallelWorker());
-		Assert(ParallelWorkerNumber <= node->shared_info->num_workers);
-
-		memcpy(&node->shared_info->sinfo[ParallelWorkerNumber],
-			   &node->incsort_info, sizeof(IncrementalSortInfo));
-	}
+	groupInfo->sortMethods |= sort_instr.sortMethod;
 }
 
 /* ----------------------------------------------------------------
@@ -435,9 +442,7 @@ switchToPresortedPrefixMode(PlanState *pstate)
 		tuplesort_performsort(node->prefixsort_state);
 
 		if (pstate->instrument != NULL)
-			instrumentSortedGroup(pstate,
-								  &node->incsort_info.prefixsortGroupInfo,
-								  node->prefixsort_state);
+			INSTRUMENT_SORT_GROUP(node, prefixsort)
 
 		if (node->bounded)
 		{
@@ -696,9 +701,7 @@ ExecIncrementalSort(PlanState *pstate)
 				tuplesort_performsort(fullsort_state);
 
 				if (pstate->instrument != NULL)
-					instrumentSortedGroup(pstate,
-										  &node->incsort_info.fullsortGroupInfo,
-										  fullsort_state);
+					INSTRUMENT_SORT_GROUP(node, fullsort)
 
 				SO_printf("Setting execution_status to INCSORT_READFULLSORT (final tuple)\n");
 				node->execution_status = INCSORT_READFULLSORT;
@@ -780,9 +783,7 @@ ExecIncrementalSort(PlanState *pstate)
 					tuplesort_performsort(fullsort_state);
 
 					if (pstate->instrument != NULL)
-						instrumentSortedGroup(pstate,
-											  &node->incsort_info.fullsortGroupInfo,
-											  fullsort_state);
+						INSTRUMENT_SORT_GROUP(node, fullsort)
 
 					SO_printf("Setting execution_status to INCSORT_READFULLSORT (found end of group)\n");
 					node->execution_status = INCSORT_READFULLSORT;
@@ -822,9 +823,7 @@ ExecIncrementalSort(PlanState *pstate)
 				SO1_printf("Sorting fullsort tuplesort with %ld tuples\n", nTuples);
 				tuplesort_performsort(fullsort_state);
 				if (pstate->instrument != NULL)
-					instrumentSortedGroup(pstate,
-										  &node->incsort_info.fullsortGroupInfo,
-										  fullsort_state);
+					INSTRUMENT_SORT_GROUP(node, fullsort)
 
 				/*
 				 * If the full sort tuplesort happened to switch into top-n
@@ -938,9 +937,7 @@ ExecIncrementalSort(PlanState *pstate)
 		tuplesort_performsort(node->prefixsort_state);
 
 		if (pstate->instrument != NULL)
-			instrumentSortedGroup(pstate,
-								  &node->incsort_info.prefixsortGroupInfo,
-								  node->prefixsort_state);
+			INSTRUMENT_SORT_GROUP(node, prefixsort)
 
 		SO_printf("Setting execution_status to INCSORT_READPREFIXSORT (found end of group)\n");
 		node->execution_status = INCSORT_READPREFIXSORT;
@@ -1026,13 +1023,13 @@ ExecInitIncrementalSort(IncrementalSort *node, EState *estate, int eflags)
 		fullsortGroupInfo->totalDiskSpaceUsed = 0;
 		fullsortGroupInfo->maxMemorySpaceUsed = 0;
 		fullsortGroupInfo->totalMemorySpaceUsed = 0;
-		fullsortGroupInfo->sortMethods = NIL;
+		fullsortGroupInfo->sortMethods = 0;
 		prefixsortGroupInfo->groupCount = 0;
 		prefixsortGroupInfo->maxDiskSpaceUsed = 0;
 		prefixsortGroupInfo->totalDiskSpaceUsed = 0;
 		prefixsortGroupInfo->maxMemorySpaceUsed = 0;
 		prefixsortGroupInfo->totalMemorySpaceUsed = 0;
-		prefixsortGroupInfo->sortMethods = NIL;
+		prefixsortGroupInfo->sortMethods = 0;
 	}
 
 	/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 6127ab5912..8d1b944472 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -2034,7 +2034,7 @@ typedef struct IncrementalSortGroupInfo
 	long		totalDiskSpaceUsed;
 	long		maxMemorySpaceUsed;
 	long		totalMemorySpaceUsed;
-	List	   *sortMethods;
+	Size		sortMethods; /* bitmask of TuplesortMethod */
 } IncrementalSortGroupInfo;
 
 typedef struct IncrementalSortInfo
diff --git a/src/include/utils/tuplesort.h b/src/include/utils/tuplesort.h
index 0e9ab4e586..96e970339c 100644
--- a/src/include/utils/tuplesort.h
+++ b/src/include/utils/tuplesort.h
@@ -61,14 +61,17 @@ typedef struct SortCoordinateData *SortCoordinate;
  * Data structures for reporting sort statistics.  Note that
  * TuplesortInstrumentation can't contain any pointers because we
  * sometimes put it in shared memory.
+ *
+ * TuplesortMethod is used in a bitmask in Increment Sort's shared memory
+ * instrumentation so needs to have each value be a separate bit.
  */
 typedef enum
 {
 	SORT_TYPE_STILL_IN_PROGRESS = 0,
-	SORT_TYPE_TOP_N_HEAPSORT,
-	SORT_TYPE_QUICKSORT,
-	SORT_TYPE_EXTERNAL_SORT,
-	SORT_TYPE_EXTERNAL_MERGE
+	SORT_TYPE_TOP_N_HEAPSORT = 2,
+	SORT_TYPE_QUICKSORT = 4,
+	SORT_TYPE_EXTERNAL_SORT = 8,
+	SORT_TYPE_EXTERNAL_MERGE = 16
 } TuplesortMethod;
 
 typedef enum
-- 
2.17.1