Re: making EXPLAIN extensible

On Thu, Mar 13, 2025 at 5:52 PM Sami Imseih <samimseih@gmail.com> wrote:
> > The validation point is an interesting one.  I agree that we don't
> > want the behavior to depend on the order in which options are
> > written.
>
> Here is what I applied on top of v6-0001 to correct this issue. Attaching it
> as a text file only as Robert may have a different opinion on how to fix
> this.
>
> I felt the best way is to create another handler for registering a validation
> function. This means we have to loop through the options list twice,
> but I don't think that is a problem.

Hmm. I thought about this at some stage in the development of these
patches, but nothing made it into the final version. I agree we should
probably do something about it, but I don't really like the idea of
calling the second hook "validate" as you've done here, because most
validation can and should be done in the regular apply hook. You only
need the second hook if you want to check the values of options
against the values of other options.

I wonder if we should just add a "plain" hook to the bottom of
ParseExplainOptionList, like post_parse_explain_options_list_hook or
something, and then EXPLAIN options that don't need this kind of
validation can just do nothing and those that do can do the usual
dance to add themselves to the hook. We could also keep it as you have
it here, with an extra handler that is called per option, but what if
some loadable module adds two new options LEFT and RIGHT and wants to
check that you don't specify LEFT and RIGHT together? Either they
register the same validate handler for both, or they register the real
validate handler for one and a no-op handler for the other. Neither of
those options seems very appealing.

-- 
Robert Haas
EDB: http://www.enterprisedb.com