Re: generic explain options v3

Robert Haas <robertmhaas(at)gmail(dot)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.

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

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

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

regards, tom lane