Re: proposal: a validator for configuration files - Mailing list pgsql-hackers

I wrote:
> I think that it might be sensible to have the following behavior:

> 1. Parse the file, where "parse" means collect all the name = value
> pairs.  Bail out if we find any syntax errors at that level of detail.
> (With this patch, we could report some or all of the syntax errors
> first.)

> 2. Tentatively apply the new custom_variable_classes setting if any.

> 3. Check to see whether all the "name"s are valid.  If not, report
> the ones that aren't and bail out.

> 4. Apply each "value".  If some of them aren't valid, report that,
> but continue, and apply all the ones that are valid.

> We can expect that the postmaster and all backends will agree on the
> results of steps 1 through 3.  They might differ as to the validity
> of individual values in step 4 (as per my example of a setting that
> depends on database_encoding), but we should never end up with a
> situation where a globally correct value is not globally applied.

I thought some more about this, and it occurred to me that it's not that
hard to foresee a situation where different backends might have
different opinions about the results of step 3, ie, different ideas
about the set of valid GUC names.  This could arise as a result of some
of them having a particular extension module loaded and others not.

Right now, whether or not you have say plpgsql loaded will not affect
your ability to do "SET plpgsql.junk = foobar" --- as long as "plpgsql"
is listed in custom_variable_classes, we'll accept the command and
create a placeholder variable for plpgsql.junk.  But it seems perfectly
plausible that we might someday try to tighten that up so that once a
module has done EmitWarningsOnPlaceholders("plpgsql"), we'll no longer
allow creation of new placeholders named plpgsql.something.  If we did
that, we could no longer assume that all backends agree on the set of
legal GUC variable names.

So that seems like an argument --- not terribly strong, but still an
argument --- for doing what I suggested next:

> The original argument for the current behavior was to avoid applying
> settings from a thoroughly munged config file, but I think that the
> checks involved in steps 1-3 would be sufficient to reject files that
> had major problems.  It's possible that step 1 is really sufficient to
> cover the issue, in which case we could drop the separate step-3 pass
> and just treat invalid GUC names as a reason to ignore the particular
> line rather than the whole file.  That would make things simpler and
> faster, and maybe less surprising too.

IOW, I'm now pretty well convinced that so long as the configuration
file is syntactically valid, we should go ahead and attempt to apply
each name = value setting individually, without allowing the invalidity
of any one name or value to prevent others from being applied.
        regards, tom lane