On Sun, Jul 26, 2009 at 7:40 PM, Tom Lane<t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> Here's the update. There are a few things that I'm not entirely happy >> with here, but not quite sure what to do about either. > > Committed with a few editorializations.
Thanks. >> - ExplainPrintPlan() is now almost trivial. It seems like there >> should be some way to get rid of this altogether, but I'm not quite >> sure how. I thought about ripping pstmt and rtable out of >> ExplainState and just storying queryDesc there. But that involves >> changing a lot of code, and while it makes some things simpler, it >> makes other parts more complex. I'm not sure whether it's a win or >> not; I'm also not sure how much brainpower it's worth spending on >> this. > > I think the problem here is that you chose to treat ExplainState.pstmt > as a parameter, when it's better considered as an internal field. > I changed it to the latter approach. Sounds fine. >> - It's becoming increasingly evident to me that the explain stuff in >> prepare.c has no business being there and should be moved to >> explain.c. I haven't done that here, but it's worth thinking about. > > I'm unconvinced. The reason that code is that way is that the > alternative would require explain.c to know quite a lot about prepared > plans, which does not seem like an improvement. I didn't consider that. As it is, prepare.c has to know quite a lot about explaining, so it may be six of one, half a dozen of the other. >> - The hack needed in ExplainLogLevel is just that. > > Yeah, I thought that was okay. We could alternatively refactor the > code so that the parameter analysis code is a separate function that > utility.c could call, but it's unclear that it's worth the trouble. OK. It seems I have quite a bit of work in front of me unbreaking the machine-readable explain patch. I started grinding through it, but it's not pretty. I'll post an updated version when I have it. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers