Re: [PATCH] Make various variables read-only (const)

I wrote:
> However, I believe this code was Alvaro's to begin with, so I'd be
> interested in his opinion on how important it is for transformRelOptions
> to allow more than one namespace per call.

> Actually, I'm kind of wondering why the code has a concept of namespaces
> at all, given the fact that it fails to store them in the resulting array.
> It seems impossible to verify after the fact that an option was given with
> the right namespace, so isn't the feature pretty much pointless?

After thinking about it some more, I realize that the intended usage
pattern is to call transformRelOptions once for each allowed namespace.
Since each call would ignore options outside its namespace, that would
result in any options with invalid namespace names being silently ignored;
so the fix was to add a parameter saying which namespaces are valid,
and then each call checks that, independently of extracting the options
it cares about.

This design seems a bit odd to me; it's certainly wasting effort, since
each namespace check after the first one is redundant. I'm inclined to
propose that we factor out the namespace validity checking into a separate
function, such that you do something like

void checkOptionNamespaces(List *defList, const char * const validnsps[])

just once, and then call transformRelOptions for each of the desired
namespaces; transformRelOptions's validnsps argument goes away. In at
least some places this looks like it would net out cleaner; for instance,
there is no good reason why callers that are trying to extract "toast"
options should have to know which other namespaces are allowed.

regards, tom lane