Re: generic explain options v3

On Tue, Jul 21, 2009 at 10:05 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Jul 21, 2009 at 7:47 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Also, I'd suggest changing the ExplainStmt struct to have a list of
>>> DefElem options instead of hard-wiring the option set at that level.
>
>> What is the advantage of that?
>
> Fewer places to change when you add a new option; in particular, not
> copyfuncs or equalfuncs.  Also, the way you are doing it is gratuitously
> unlike every other command that has similar issues to deal with.
> Everybody else parses their DefElem list at execution time.  I think
> you should have the legacy ANALYZE and VERBOSE syntax elements generate
> DefElem list members that get examined at execution.

Not having to touch copyfuncs or equalfuncs for future options is a
definite plus, so I'll rework along these lines.

> BTW, I see that your "explain refactoring" patch is marked ready
> for committer, but is it actually sane to apply it before the other
> two?

I think so. It's all code cleanup, with no behavioral changes, and is
intended to contain only the stuff that seemed to me as thought it
would still be worth doing even if the rest of the patch set were
rejected.

...Robert