Re: Auto-explain patch

Dean Rasheed <dean_rasheed@hotmail.com> writes:
> This new version fixes that and also includes a little patch to psql so that it
> ignores any backend notices during tab-completion, which otherwise just get
> in the way. Trace during tab-completion still goes to the server log, if enabled,
> since this might actually be useful for debugging psql.

Comments:

I do not think that you should invent a new elog level for this, and
especially not one that is designed to send unexpected messages to the
client.  Having to kluge tab completion like that is just a signal that
you're going to break a lot of other clients too.  It seems to me that
the right behavior for auto-explain messages is to go only to the log by
default, which means that LOG is already a perfectly good elog level for
auto-explain messages.

Drop the query_string addition to PlannedStmt --- there are other ways
you can get that string in CVS HEAD.  I don't think that planner_time
belongs there either.  It would be impossible to define a principled way
to compare two PlannedStmts for equality with that in there.  Nor do I
see the point of doing it the way you're doing it.  Why don't you just
log the slow planning cycle immediately upon detecting it in planner()?
I don't see that a slow planning cycle has anything necessarily to do
with a slow execution cycle, so IMHO they ought to just get logged
independently.

Please do not export ExplainState --- that's an internal matter for
explain.c.  Export some wrapper function with a cleaner API than
explain_outNode, instead.
        regards, tom lane