Make EXPLAIN ANALYZE of HashAgg more similar to Hash Join

There were various unnecessary differences between Hash Agg's EXPLAIN
ANALYZE output and Hash Join's.  Here we modify the Hash Agg output so
that it's better aligned to Hash Join's.

The following changes have been made:
1. Start batches counter at 1 instead of 0.
2. Always display the "Batches" property, even when we didn't spill to
   disk.
3. Use the text "Batches" instead of "HashAgg Batches" for text format.
4. Use the text "Memory Usage" instead of "Peak Memory Usage" for text
   format.
5. Include "Batches" before "Memory Usage" in both text and non-text
   formats.

In passing also modify the "Planned Partitions" property so that we show
it regardless of if the value is 0 or not for non-text EXPLAIN formats.
This was pointed out by Justin Pryzby and probably should have been part
of 40efbf870.

Reviewed-by: Justin Pryzby, Jeff Davis
Discussion: https://postgr.es/m/CAApHDvrshRnA6C0VFnu7Fb9TVvgGo80PUMm5+2DiaS1gEkPvtw@mail.gmail.com
Backpatch-through: 13, where HashAgg batching was introduced

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index a283e4d45c84bd3dd86a0f807627ef8db2b62d39..54e3797a15b6260fcfb776325dfb434566d71476 100644 (file)
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3059,21 +3059,19 @@ show_hashagg_info(AggState *aggstate, ExplainState *es)
    if (es->format != EXPLAIN_FORMAT_TEXT)
    {
 
-       if (es->costs && aggstate->hash_planned_partitions > 0)
-       {
+       if (es->costs)
            ExplainPropertyInteger("Planned Partitions", NULL,
                                   aggstate->hash_planned_partitions, es);
-       }
 
        if (!es->analyze)
            return;
 
        /* EXPLAIN ANALYZE */
+       ExplainPropertyInteger("HashAgg Batches", NULL,
+                              aggstate->hash_batches_used, es);
        ExplainPropertyInteger("Peak Memory Usage", "kB", memPeakKb, es);
        ExplainPropertyInteger("Disk Usage", "kB",
                               aggstate->hash_disk_used, es);
-       ExplainPropertyInteger("HashAgg Batches", NULL,
-                              aggstate->hash_batches_used, es);
    }
    else
    {
@@ -3099,13 +3097,13 @@ show_hashagg_info(AggState *aggstate, ExplainState *es)
        else
            appendStringInfoString(es->str, "  ");
 
-       appendStringInfo(es->str, "Peak Memory Usage: " INT64_FORMAT "kB",
-                        memPeakKb);
+       appendStringInfo(es->str, "Batches: %d  Memory Usage: " INT64_FORMAT "kB",
+                        aggstate->hash_batches_used, memPeakKb);
 
-       if (aggstate->hash_batches_used > 0)
-           appendStringInfo(es->str, "  Disk Usage: " UINT64_FORMAT "kB  HashAgg Batches: %d",
-                            aggstate->hash_disk_used,
-                            aggstate->hash_batches_used);
+       /* Only display disk usage if we spilled to disk */
+       if (aggstate->hash_batches_used > 1)
+           appendStringInfo(es->str, "  Disk Usage: " UINT64_FORMAT "kB",
+                            aggstate->hash_disk_used);
        appendStringInfoChar(es->str, '\n');
    }
 
@@ -3130,21 +3128,22 @@ show_hashagg_info(AggState *aggstate, ExplainState *es)
            {
                ExplainIndentText(es);
 
-               appendStringInfo(es->str, "Peak Memory Usage: " INT64_FORMAT "kB",
-                                memPeakKb);
+               appendStringInfo(es->str, "Batches: %d  Memory Usage: " INT64_FORMAT "kB",
+                                hash_batches_used, memPeakKb);
 
-               if (hash_batches_used > 0)
-                   appendStringInfo(es->str, "  Disk Usage: " UINT64_FORMAT "kB  HashAgg Batches: %d",
-                                    hash_disk_used, hash_batches_used);
+               /* Only display disk usage if we spilled to disk */
+               if (hash_batches_used > 1)
+                   appendStringInfo(es->str, "  Disk Usage: " UINT64_FORMAT "kB",
+                                    hash_disk_used);
                appendStringInfoChar(es->str, '\n');
            }
            else
            {
+               ExplainPropertyInteger("HashAgg Batches", NULL,
+                                      hash_batches_used, es);
                ExplainPropertyInteger("Peak Memory Usage", "kB", memPeakKb,
                                       es);
                ExplainPropertyInteger("Disk Usage", "kB", hash_disk_used, es);
-               ExplainPropertyInteger("HashAgg Batches", NULL,
-                                      hash_batches_used, es);
            }
 
            if (es->workers_state)

diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index bbfc4af1ec9cb1c581a1a49da4eeee6b24d662f7..7d7bfa945605be57e0dd89cd2c3f5f1d9c37e1aa 100644 (file)
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3641,6 +3641,9 @@ ExecInitAgg(Agg *node, EState *estate, int eflags)
        find_hash_columns(aggstate);
        build_hash_tables(aggstate);
        aggstate->table_filled = false;
+
+       /* Initialize this to 1, meaning nothing spilled, yet */
+       aggstate->hash_batches_used = 1;
    }
 
    /*