Re: explain HashAggregate to report bucket and memory stats

On Fri, Mar 06, 2020 at 09:58:59AM -0800, Andres Freund wrote:
> ...
>
>> +    }
>> +    else if (!inst->nbuckets)
>> +        ; /* Do nothing */
>> +    else
>> +    {
>> +        if (inst->nbuckets_original != inst->nbuckets)
>> +        {
>> +            ExplainIndentText(es);
>> +            appendStringInfo(es->str,
>> +                        "Buckets: %ld (originally %ld)",
>> +                        inst->nbuckets,
>> +                        inst->nbuckets_original);
>> +        }
>> +        else
>> +        {
>> +            ExplainIndentText(es);
>> +            appendStringInfo(es->str,
>> +                        "Buckets: %ld",
>> +                        inst->nbuckets);
>> +        }
>> +
>> +        if (es->analyze)
>> +            appendStringInfo(es->str,
>> +                    "  Memory Usage: hashtable: %ldkB, tuples: %ldkB",
>> +                    spacePeakKb_hash, spacePeakKb_tuples);
>> +        appendStringInfoChar(es->str, '\n');
>
>I'm not sure I like the alternative output formats here. All the other
>fields are separated with a comma, but the original size is in
>parens. I'd probably just format it as "Buckets: %lld " and then add
>", Original Buckets: %lld" when differing.
>

FWIW this copies hashjoin precedent, which does this:

     appendStringInfo(es->str,
                      "Buckets: %d (originally %d)  Batches: %d (originally %d)  Memory Usage: %ldkB\n",
                      hinstrument.nbuckets,
             ...

I agree it's not ideal, but maybe let's not invent new ways to format
the same type of info.


regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services