Thanks for the feedback, and sorry for my delay in replying (I was on holiday).
> Tom Lane wrote: > > 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. The more I thought about this, the more I thought that it was OTT to add a new elog level just for this, so I agree it should probably just go to the LOG elog level. I don't agree with your reasoning on tab-completion though. I actually think that this is a signal of a broken client. If the user sets client_min_messages to LOG or lower, and has any of the other logging or debugging parameters enabled, the output tramples all over the suggested completions. I don't think the average user is interested in log/debug output from the queries psql does internally during tab-completion. So IMHO the tab-completion 'kludge', is still worthwhile, regardless of the rest of the patch. > Drop the query_string addition to PlannedStmt --- there are other ways > you can get that string in CVS HEAD. OK. What is the best way to get this string now? Are you referring to debug_query_string, or is there another way? > 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. Makes sense. > 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 OK, that's much neater. I'll try to rework this ASAP but I understand if it's too late for this commitfest. Cheers, Dean. _________________________________________________________________ Win a voice over part with Kung Fu Panda & Live Search and 100’s of Kung Fu Panda prizes to win with Live Search http://clk.atdmt.com/UKM/go/107571439/direct/01/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers