Re: proposal: a validator for configuration files

On Jun16, 2011, at 17:23 , Alexey Klyukin wrote:
> On Jun 16, 2011, at 2:34 PM, Florian Pflug wrote:
>> The first problem I ran into when I tried to test this is that it *only*
>> reports multiple errors during config file reload on SIHUP, not during
>> postmaster startup. I guess it's been done that way because we
>> ereport(ERROR,..) not ereport(LOG,...) during postmaster startup, so it's
>> not immediatly clear how to report multiple errors. But that proplem
>> seems solvable. What if you ereport(LOG,..)ed the individual errors during
>> postmaster startup, and then emitted an ereport(ERROR) at the end if
>> errors occurred? The ERROR could either repeat the first error that was
>> encountered, or simply say "config file contains errors".
>
> Makes sense. One problem I came across is that set_config_option from guc.c
> sets the elevel by itself. I've changed it to emit LOG errors on PGC_S_FILE
> source, apparently all the callers of this function with this source are from
> guc-file.l, so hopefully I won't break anything with this change.

Hm, wouldn't a test for "context == PGC_POSTMASTER" be more appropriate?

>> I see that you basically replaced "goto cleanup..." in both ParseConfigFp()
>> and ProcessConfigFile() with "++errorcount", and arranged for ParseConfigFp()
>> to return false, and for ProcessConfigFile() to skip the GUC updates if
>> "errorcount > 0". The actual value of errorcount is never inspected. The value
>> is also wrong in the case of include files containing more than error, since
>> ParseConfigFp() simply increments errorcount by one for each failed
>> ParseConfigFile() of an included file.
>>
>> I thus suggest that you replace "errorcount" with a boolean "erroroccurred".
>
> I can actually pass errorcount down from the ParseConfigFp() to report the total
> number of errors (w/ the include files) at the end of ProcessConfigFile if there is
> any interest in having the number of errors reported to a user. If not - I'll change
> it to boolean.

Nah, just use a boolean, unless you have concrete plans to actually use the errorcount
for something other than test a la "errorcount > 0".

best regards,
Florian Pflug