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