Re: proposal: a validator for configuration files

Excerpts from Alexey Kluykin's message of mar jun 21 07:43:02 -0400 2011:

> > Another benefit of removing the check is that it reduces code complexity. Maybe
> > not when measured in line counts, but it's one less outside factor that changes
> > ProcessConfigFiles()'s behaviour and thus one thing less you need to think when
> > you modify that part again in the future. Again, it's a small benefit, but IMHO
> > it still outweights the benefit.
>
> While I agree that making the code potentially less bug prone is a good idea,
> I don't agree with the statement that since DEBUG2 output gets rarely turned
> on we can make it less usable, hoping that nobody would use in production.

I would have sided with Florian on this issue, but Tom commented
otherwise somewhere, and I think his rationale makes sense, so I left
the patch as Alexey had it.

I also tweaked the patch so that it really stops processing after 100
errors, say if you include file A which has 50 errors and then file B
which has another 50 errors; instead of looking for 98 more errors, we
just give up at that point. It makes more sense to me anyway, and it
doesn't seem to add any code complexity. Also, if you have already seen
98 errors, it doesn't make sense to let another file contain 100 errors
more, so with this version, that 98 is passed down so that only 2 more
errors are allowed. I had to touch the other callers of ParseConfigFile
but it is clean enough.

I also made the code barf when bailing out of one of those loops. One
strange thing here is that you could get two such messages; say if a
file has 100 parse errors and there are also valid lines that contain
bogus settings (foo = bar). I don't find this to be too problematic,
and I think fixing it would be excessively annoying.

For example, a bogus run would end like this:

95 LOG: syntax error in file "/pgsql/install/HEAD/data/postgresql.conf" line 4, near end of line
96 LOG: syntax error in file "/pgsql/install/HEAD/data/postgresql.conf" line 41, near end of line
97 LOG: syntax error in file "/pgsql/install/HEAD/data/postgresql.conf" line 104, near end of line
98 LOG: syntax error in file "/pgsql/install/HEAD/data/postgresql.conf" line 156, near end of line
99 LOG: syntax error in file "/pgsql/install/HEAD/data/postgresql.conf" line 208, near end of line
100 LOG: syntax error in file "/pgsql/install/HEAD/data/postgresql.conf" line 260, near end of line
101 LOG: too many errors found, stopped processing file "/pgsql/install/HEAD/data/postgresql.conf"
102 LOG: unrecognized configuration parameter "plperl.err"
103 LOG: unrecognized configuration parameter "this1"
104 LOG: too many errors found, stopped processing file "/pgsql/install/HEAD/data/postgresql.conf"
105 FATAL: errors detected while parsing configuration files

One thing I don't like is that those "unrecognized configuration
parameter" messages don't say what file those parameters were found in.
That's material for a different patch however.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support