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
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 |