From 42634353f44ab44a06ffe4ae36a0dcbb848408ca Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 1 Jan 2020 13:09:33 -0600
Subject: [PATCH v1 1/2] refactor: show_hinstrument and avoid showing memory
 use if not verbose..

This changes explain analyze at least for Hash(join), but doesn't affect
regression tests, since all the HashJoin tests seem to run explain without
analyze, so nbatch=0, and no stats are shown.

But for future patch to show stats for HashAgg (for which nbatch=1, always), we
want to show buckets in explain analyze, but don't want to show memory, since
it's machine-specific.
---
 src/backend/commands/explain.c | 79 +++++++++++++++++++++++++++---------------
 1 file changed, 52 insertions(+), 27 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index f523adb..11b5857 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -103,6 +103,7 @@ static void show_sortorder_options(StringInfo buf, Node *sortexpr,
 static void show_tablesample(TableSampleClause *tsc, PlanState *planstate,
 							 List *ancestors, ExplainState *es);
 static void show_sort_info(SortState *sortstate, ExplainState *es);
+static void show_hinstrument(ExplainState *es, HashInstrumentation *h);
 static void show_hash_info(HashState *hashstate, ExplainState *es);
 static void show_tidbitmap_info(BitmapHeapScanState *planstate,
 								ExplainState *es);
@@ -2713,43 +2714,67 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 		}
 	}
 
-	if (hinstrument.nbatch > 0)
+	show_hinstrument(es, &hinstrument);
+}
+
+/*
+ * Show hash bucket stats and (optionally) memory.
+ */
+static void
+show_hinstrument(ExplainState *es, HashInstrumentation *h)
+{
+	long		spacePeakKb = (h->space_peak + 1023) / 1024;
+
+	// Currently, this isn't shown for explain of hash(join) since nbatch=0 without analyze
+	// But, it's shown for hashAgg since nbatch=1, always.
+	// Need to 1) avoid showing memory use if !analyze; and, 2) avoid memory use if not verbose
+
+	if (h->nbatch <= 0)
+		return;
+	/* This avoids showing anything if it's explain without analyze; should we just check that, instead ? */
+	if (!es->analyze)
+		return;
+
+	if (es->format != EXPLAIN_FORMAT_TEXT)
+	{
+		ExplainPropertyInteger("Hash Buckets", NULL,
+							   h->nbuckets, es);
+		ExplainPropertyInteger("Original Hash Buckets", NULL,
+							   h->nbuckets_original, es);
+		ExplainPropertyInteger("Hash Batches", NULL,
+							   h->nbatch, es);
+		ExplainPropertyInteger("Original Hash Batches", NULL,
+							   h->nbatch_original, es);
+		ExplainPropertyInteger("Peak Memory Usage", "kB",
+							   spacePeakKb, es);
+	}
+	else
 	{
-		long		spacePeakKb = (hinstrument.space_peak + 1023) / 1024;
 
-		if (es->format != EXPLAIN_FORMAT_TEXT)
-		{
-			ExplainPropertyInteger("Hash Buckets", NULL,
-								   hinstrument.nbuckets, es);
-			ExplainPropertyInteger("Original Hash Buckets", NULL,
-								   hinstrument.nbuckets_original, es);
-			ExplainPropertyInteger("Hash Batches", NULL,
-								   hinstrument.nbatch, es);
-			ExplainPropertyInteger("Original Hash Batches", NULL,
-								   hinstrument.nbatch_original, es);
-			ExplainPropertyInteger("Peak Memory Usage", "kB",
-								   spacePeakKb, es);
-		}
-		else if (hinstrument.nbatch_original != hinstrument.nbatch ||
-				 hinstrument.nbuckets_original != hinstrument.nbuckets)
-		{
+		if (h->nbatch_original != h->nbatch ||
+			 h->nbuckets_original != h->nbuckets) {
 			ExplainIndentText(es);
 			appendStringInfo(es->str,
-							 "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: %ldkB\n",
-							 hinstrument.nbuckets,
-							 hinstrument.nbuckets_original,
-							 hinstrument.nbatch,
-							 hinstrument.nbatch_original,
-							 spacePeakKb);
+						"Buckets: %d (originally %d)   Batches: %d (originally %d)",
+						h->nbuckets,
+						h->nbuckets_original,
+						h->nbatch,
+						h->nbatch_original);
 		}
 		else
 		{
 			ExplainIndentText(es);
 			appendStringInfo(es->str,
-							 "Buckets: %d  Batches: %d  Memory Usage: %ldkB\n",
-							 hinstrument.nbuckets, hinstrument.nbatch,
-							 spacePeakKb);
+						"Buckets: %d  Batches: %d",
+						h->nbuckets,
+						h->nbatch);
 		}
+
+		if (es->verbose && es->analyze)
+			appendStringInfo(es->str,
+					"  Memory Usage: %ldkB",
+					spacePeakKb);
+		appendStringInfoChar(es->str, '\n');
 	}
 }
 
-- 
2.7.4