Date: Fri, 6 Mar 2020 20:43:07 +0100
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Andres Freund <andres@anarazel.de>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	pgsql-hackers@lists.postgresql.org,
	Jeff Janes <jeff.janes@gmail.com>
Subject: Re: explain HashAggregate to report bucket and memory stats
Message-ID: <20200306194307.qeobgf3bklqodecv@development>
References: <20200103161925.GM12066@telsasoft.com>
 <20200203145301.53mozz7gcdaklnjc@alap3.anarazel.de>
 <20200216000220.GF31889@telsasoft.com>
 <20200216175307.GJ31889@telsasoft.com>
 <20200219201037.GA21433@telsasoft.com>
 <20200222215335.bu4kky7el4nccls5@development>
 <20200225033532.GX31889@telsasoft.com>
 <20200301154540.GL29456@telsasoft.com>
 <20200306175859.d56ohskarwldyrrw@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20200306175859.d56ohskarwldyrrw@alap3.anarazel.de>
Precedence: bulk

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