On Sat, Feb 22, 2020 at 10:53:35PM +0100, Tomas Vondra wrote:
> 1) explain.c API
>
> The functions in explain.c (even the static ones) follow the convention
> that the last parameter is (ExplainState *es). I think we should stick
> to this, so the new parameters should be added before it.
I found it weird to have the "constant" arguments at the end rather than at the
beginning. (Also, these don't follow that convention: show_buffer_usage
ExplainSaveGroup ExplainRestoreGroup ExplainOneQuery ExplainPrintJIT).
But done.
> Also, the first show_grouping_sets should be renamed to aggstate to make
> it consistent with the type change.
The prototype wasn't updated - fixed.
> 2) The hash_instrumentation is a bit inconsistent with what we already
> have ..HashTableInstrumentation..
Thanks for thinking of a better name.
> 5) I think the explain for grouping sets need a rething. Teh function
> show_grouping_set_keys was originally meant to print just the keys, but
> now it's also printing the hash table stats. IMO we need a new function
> printing a grouping set info - calling show_grouping_set_keys to print
> the keys, but then also printing the extra hashtable info.
I renamed it, and did the rest in a separate patch for now, since I'm only
partially convinced it's an improvement.
> 6) subplan explain
>
> That is, there's no indication why would this use a hash table, because
> the "hashed subplan" is included only in verbose mode:
Need to think about that..
> Not sure if this is an issue, maybe it's fine. But it's definitely
> strange that we only print memory info in verbose mode - IMHO it's much
> more useful info than the number of buckets etc.
You're right that verbose isn't right for this.
I wrote patches creating new explain options to allow stable output of "explain
analyze", by avoiding Memory/Disk. The only other way to handle it seems to be
to avoid "explain analyze" in regression tests, which is what's in common
practice anyway, so did that instead.
I also fixed wrong output and wrong non-text formatting for grouping sets,
tweaked output for subplan, and broke style rules less often.
--
Justin