Re: ExplainProperty* and units

Hi,

On 2018-03-13 17:27:40 -0700, Andres Freund wrote:
> Hi,
>
> while adding EXPLAIN support for JITing (displaying time spent etc), I
> got annoyed by the amount of duplication required. There's a fair amount
> of
> if (es->format == EXPLAIN_FORMAT_TEXT)
> appendStringInfo(es->str, "Execution time: %.3f ms\n",
> 1000.0 * totaltime);
> else
> ExplainPropertyFloat("Execution Time", 1000.0 * totaltime,
> which is fairly redundant.
>
> In the attached *POC* patch I've added a 'unit' parameter to the numeric
> ExplainProperty* functions, which EXPLAIN_FORMAT_TEXT adds to the
> output. This can avoid the above and other similar branches (of which
> the JIT patch would add a number).

If we do this, and I think we should, I'm inclined to also commit a
patch that renames

/*
* Explain a long-integer-valued property.
*/
void
ExplainPropertyLong(const char *qlabel, const char *unit, long value,
ExplainState *es)
{
char buf[32];

snprintf(buf, sizeof(buf), "%ld", value);
ExplainProperty(qlabel, unit, buf, true, es);
}

and changes its argument type. Because passing long is just plain
unhelpful for 32bit platforms and windows. We should just always use
64bits here. Only thing I wonder is if we shouldn't just *remove*
ExplainPropertyLong and make ExplainPropertyInteger accept 64bits of
input - the effort of passing and printing a 64bit integer will never be
relevant for explain.

- Andres