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