Re: machine-readable explain output v4 - Mailing list pgsql-hackers

From Tom Lane
Subject Re: machine-readable explain output v4
Date
Msg-id 26060.1249257468@sss.pgh.pa.us
Whole thread Raw
In response to Re: machine-readable explain output v4  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: machine-readable explain output v4  (Andres Freund <andres@anarazel.de>)
Re: machine-readable explain output v4  (Robert Haas <robertmhaas@gmail.com>)
Re: machine-readable explain output v4  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
>>> The reason for this regression is that Tom asked me to change
>>> ExplainStmt to just carry a list of nodes and to do all the parsing in
>>> ExplainQuery. �Unfortunately, the TupleDesc is constructed by
>>> ExplainResultDesc() which can't trivially be changed to take an
>>> ExplainState, because UtilityTupleDescriptor() also wants to call it.
>>> We could possibly fix this by a hack similar to the one we already
>>> added to GetCommandLogLevel(), but I haven't done that here.

I don't see anything particularly wrong with having ExplainResultDesc
do the same kind of thing GetCommandLogLevel is doing.

The alternative is to have it call a function that extracts all the
parameters into an ExplainState, but the thing I have against that is
that it makes for a larger cross-section of user mistakes that will
result in throwing an elog() before we actually reach execution.
I didn't want GetCommandLogLevel throwing any errors it doesn't actually
have to, because that would interfere with logging of statements that
could perfectly well get logged normally.  I'm not sure if there are any
comparable reasons to not want errors from UtilityTupleDescriptor, but
there could well be.

>> - The regression tests are gone?

> Tom added some that look adequate to me to create_index.sql, as a
> separate commit, so I don't think I need to do this in my patch any
> more.  Maybe some of those examples should be changed to output JSON
> or XML, though, but I'd rather leave this up to Tom's discretion on
> commit because I think he has opinions about this and I think my
> chances of guessing what they are are low.

Well, of course the existing tests are not going to exercise XML or
JSON output format.  Dunno how much we care.  I had supposed that
XML or JSON would always emit all the fields and leave it to the
recipient to suppress what they don't want.  If we want to have
platform-independent regression tests then we'd need to make the
COSTS option effective for XML/JSON format --- do we want that?

> I'm not sure what you mean by "any more".  I don't think any version
> of these patches I ever submitted did otherwise, nor do I think it's
> particularly valuable.  Costs are implicitly in units of
> PostgreSQL-costing and times are implicitly in units of milliseconds,
> just as they are in the text format.  Changing this would require
> clients to strip off the trailing 'ms' before converting to a
> floating-point number, which seems like an irritation with no
> corresponding benefit.

I agree with not labeling these numbers.  We definitely can't label
the costs with anything useful, and labeling runtimes would be a
bit inconsistent.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Joe Conway
Date:
Subject: change in timestamp output from 8.3 to 8.4
Next
From: Tom Lane
Date:
Subject: Re: change in timestamp output from 8.3 to 8.4