On Thu, Mar 13, 2025 at 5:52 PM Sami Imseih <samims...@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