0002-Use-Planning-section-to-report-memory-per-A-20240118.patch

From 4637dd99b19bf8e3670a12850589aad235350a5a Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
Date: Thu, 18 Jan 2024 15:04:53 +0530
Subject: [PATCH 2/2] Use Planning section to report memory per Alvaro's
 comment

Buffer usage during planning is reported under "Planning" section. Use
the same section to report "Memory".

Note to committer:
This commit should be merged with the previous one before committing.

Author: Ashutosh Bapat
---
 src/backend/commands/explain.c        | 85 ++++++++++++++++++---------
 src/test/regress/expected/explain.out | 20 +++----
 2 files changed, 68 insertions(+), 37 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 2fa93998a3..7e286f5020 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -119,11 +119,12 @@ static void show_instrumentation_count(const char *qlabel, int which,
 static void show_foreignscan_info(ForeignScanState *fsstate, ExplainState *es);
 static void show_eval_params(Bitmapset *bms_params, ExplainState *es);
 static const char *explain_get_index_name(Oid indexId);
-static void show_buffer_usage(ExplainState *es, const BufferUsage *usage,
+static bool show_buffer_usage(ExplainState *es, const BufferUsage *usage,
 							  bool planning);
 static void show_wal_usage(ExplainState *es, const WalUsage *usage);
-static void show_planner_memory(ExplainState *es,
-								const MemoryContextCounters *mem_counts);
+static bool show_planner_memory(ExplainState *es,
+								const MemoryContextCounters *mem_counts,
+								bool show_planning);
 static void ExplainIndexScanDetails(Oid indexid, ScanDirection indexorderdir,
 									ExplainState *es);
 static void ExplainScanTarget(Scan *plan, ExplainState *es);
@@ -645,11 +646,22 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
 	/* Create textual dump of plan tree */
 	ExplainPrintPlan(es, queryDesc);
 
-	/* Show buffer usage in planning */
-	if (bufusage)
+	/* Show buffer and/or memory usage in planning */
+	if (bufusage || mem_counts)
 	{
+		bool		showed_planning = false;
+
 		ExplainOpenGroup("Planning", "Planning", true, es);
-		show_buffer_usage(es, bufusage, true);
+
+		if (bufusage)
+			showed_planning = show_buffer_usage(es, bufusage, true);
+
+		if (mem_counts)
+			showed_planning |= show_planner_memory(es, mem_counts, !showed_planning);
+
+		if (showed_planning)
+			es->indent--;
+
 		ExplainCloseGroup("Planning", "Planning", true, es);
 	}
 
@@ -660,13 +672,6 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
 		ExplainPropertyFloat("Planning Time", "ms", 1000.0 * plantime, 3, es);
 	}
 
-	if (es->memory && mem_counts != NULL)
-	{
-		ExplainOpenGroup("Planner Memory", "Planner Memory", true, es);
-		show_planner_memory(es, mem_counts);
-		ExplainCloseGroup("Planner Memory", "Planner Memory", true, es);
-	}
-
 	/* Print info about runtime of triggers */
 	if (es->analyze)
 		ExplainPrintTriggers(es, queryDesc);
@@ -3583,10 +3588,20 @@ explain_get_index_name(Oid indexId)
 
 /*
  * Show buffer usage details.
+ *
+ * When reporting in TEXT format this function opens "planning" section if only there are counts to
+ * be reported for planning. The function returns true if the section was
+ * opened, false otherwise. The function does not close the section. The caller is expected to use the return value to
+ * close the section if required.
+ *
+ * When reporting in non-TEXT format the caller is expected to open and close the section,
+ * if required. Hence the function always returns false for non-TEXT formats.
  */
-static void
+static bool
 show_buffer_usage(ExplainState *es, const BufferUsage *usage, bool planning)
 {
+	bool		show_planning = false;
+
 	if (es->format == EXPLAIN_FORMAT_TEXT)
 	{
 		bool		has_shared = (usage->shared_blks_hit > 0 ||
@@ -3605,12 +3620,10 @@ show_buffer_usage(ExplainState *es, const BufferUsage *usage, bool planning)
 										!INSTR_TIME_IS_ZERO(usage->local_blk_write_time));
 		bool		has_temp_timing = (!INSTR_TIME_IS_ZERO(usage->temp_blk_read_time) ||
 									   !INSTR_TIME_IS_ZERO(usage->temp_blk_write_time));
-		bool		show_planning = (planning && (has_shared ||
-												  has_local || has_temp ||
-												  has_shared_timing ||
-												  has_local_timing ||
-												  has_temp_timing));
 
+		show_planning = (planning && (has_shared || has_local || has_temp ||
+									  has_shared_timing || has_local_timing ||
+									  has_temp_timing));
 		if (show_planning)
 		{
 			ExplainIndentText(es);
@@ -3715,9 +3728,6 @@ show_buffer_usage(ExplainState *es, const BufferUsage *usage, bool planning)
 			}
 			appendStringInfoChar(es->str, '\n');
 		}
-
-		if (show_planning)
-			es->indent--;
 	}
 	else
 	{
@@ -3763,6 +3773,8 @@ show_buffer_usage(ExplainState *es, const BufferUsage *usage, bool planning)
 								 3, es);
 		}
 	}
+
+	return show_planning;
 }
 
 /*
@@ -3805,27 +3817,46 @@ show_wal_usage(ExplainState *es, const WalUsage *usage)
 
 /*
  * Show planner's memory usage details.
+ *
+ * When reporting in TEXT format this function opens the "planning" section if only it was not opened already. The function returns true if it opened the section,
+ * false otherwise. The caller is expected to use the return value to
+ * close the section if required.
+ *
+ * When reporting in non-TEXT format the caller is expected to open and close the section,
+ * if required. Hence the function always returns false for non-TEXT formats.
  */
-static void
+static bool
 show_planner_memory(ExplainState *es,
-					const MemoryContextCounters *mem_counts)
+					const MemoryContextCounters *mem_counts, bool show_planning)
 {
 	if (es->format == EXPLAIN_FORMAT_TEXT)
 	{
+		if (show_planning)
+		{
+			ExplainIndentText(es);
+			appendStringInfoString(es->str, "Planning:\n");
+			es->indent++;
+		}
+
+		ExplainIndentText(es);
 		appendStringInfo(es->str,
-						 "Planner Memory: used=%lld bytes allocated=%lld bytes",
+						 "Memory: used=%lld bytes, allocated=%lld bytes",
 						 (long long) (mem_counts->totalspace - mem_counts->freespace),
 						 (long long) mem_counts->totalspace);
 		appendStringInfoChar(es->str, '\n');
 	}
 	else
 	{
-		ExplainPropertyInteger("Used", "bytes",
+		show_planning = false;
+
+		ExplainPropertyInteger("Memory Used", "bytes",
 							   mem_counts->totalspace - mem_counts->freespace,
 							   es);
-		ExplainPropertyInteger("Allocated", "bytes",
+		ExplainPropertyInteger("Memory Allocated", "bytes",
 							   mem_counts->totalspace, es);
 	}
+
+	return show_planning;
 }
 
 
diff --git a/src/test/regress/expected/explain.out b/src/test/regress/expected/explain.out
index 86bfdfd29e..55694505a7 100644
--- a/src/test/regress/expected/explain.out
+++ b/src/test/regress/expected/explain.out
@@ -331,15 +331,15 @@ select explain_filter('explain (memory) select * from int8_tbl i8');
                      explain_filter                      
 ---------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N)
- Planner Memory: used=N bytes allocated=N bytes
+   Memory: used=N bytes, allocated=N bytes
 (2 rows)
 
 select explain_filter('explain (memory, analyze) select * from int8_tbl i8');
                                         explain_filter                                         
 -----------------------------------------------------------------------------------------------
  Seq Scan on int8_tbl i8  (cost=N.N..N.N rows=N width=N) (actual time=N.N..N.N rows=N loops=N)
+   Memory: used=N bytes, allocated=N bytes
  Planning Time: N.N ms
- Planner Memory: used=N bytes allocated=N bytes
  Execution Time: N.N ms
 (4 rows)
 
@@ -356,10 +356,10 @@ select explain_filter('explain (memory, summary, format yaml) select * from int8
      Total Cost: N.N          +
      Plan Rows: N             +
      Plan Width: N            +
-   Planning Time: N.N         +
-   Planner Memory:            +
-     Used: N                  +
-     Allocated: N
+   Planning:                  +
+     Memory Used: N           +
+     Memory Allocated: N      +
+   Planning Time: N.N
 (1 row)
 
 select explain_filter('explain (memory, analyze, format json) select * from int8_tbl i8');
@@ -382,11 +382,11 @@ select explain_filter('explain (memory, analyze, format json) select * from int8
        "Actual Rows": N,           +
        "Actual Loops": N           +
      },                            +
-     "Planning Time": N.N,         +
-     "Planner Memory": {           +
-       "Used": N,                  +
-       "Allocated": N              +
+     "Planning": {                 +
+       "Memory Used": N,           +
+       "Memory Allocated": N       +
      },                            +
+     "Planning Time": N.N,         +
      "Triggers": [                 +
      ],                            +
      "Execution Time": N.N         +
-- 
2.25.1