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 <alvhe...@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
pg_parser_continue_on_error_v3.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
