Re: Auto-explain patch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Dean Rasheed <dean_rasheed(at)hotmail(dot)com>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Auto-explain patch
Date: 2008-07-26 23:44:16
Message-ID: 5281.1217115856@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dean Rasheed <dean_rasheed(at)hotmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2008-07-27 00:28:47 Search for version-numbered tclsh?
Previous Message Tom Lane 2008-07-26 23:09:45 Re: Review: DTrace probes (merged version) ver_03