On Tue, Jan 24, 2017 at 6:17 PM, Michael Paquier <michael.paqu...@gmail.com> wrote:
> On Mon, Jan 23, 2017 at 5:13 PM, Haribabu Kommi > <kommi.harib...@gmail.com> wrote: > > On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> * I'm not exactly convinced that the way you approached the error > message > >> reporting, ie duplicating the logged message, is good. In particular > >> this results in localizing the strings reported in pg_hba_rules.error, > >> which is exactly opposite to the decision we reached for the > >> pg_file_settings view. What's the reasoning for deciding that this > >> view should contain localized strings? (More generally, we found in > >> the pg_file_settings view that we didn't always want to use exactly > >> the same string that was logged, anyway.) > > > > Actually there is no particular reason to display the localized strings, > > Just thought that it may be useful to the user if it get displayed in > their > > own language. And also doing this way will reduce the error message > > duplicate in the code that is used for display in the view and writing it > > into the log file. > > Perhaps consistency would not hurt and something like > record_config_file_error() could be done to save the error parsing > error. What's actually the problem with localized strings exposed in a > system view? Encoding conflicts? > > >> * Also, there seems to be a lot of ereports remaining unconverted, > >> eg the "authentication file token too long" error. One of the things > >> we wanted pg_file_settings to be able to do was finger pretty much any > >> mistake in the config file, including syntax errors. It seems like > >> it'd be a shame if pg_hba_rules is unable to help with that. You > >> should be able to fill in line number and error even if the line is > >> too mangled to be able to populate the other fields sanely. > > > > The two errors that are missed are, "could not open secondary > authentication > > file" > > and "authentication file token too long" errors. For these two cases, the > > server > > is not throwing any error, it just logs the message and continues. Is it > > fine to add > > these these two cases as errors in the view? > > Missed those ones during the initial review... It would be a good idea > to include them to track problems. > The above mentioned two error logs that occur in the tokenize_file function. Currently during the loading of pg_hba.conf file, it just logs the above two problems and continue to load the file. Currently, I added the errors for the cases, where the server will stop proceeding because of these errors. Those are mostly in parse_hba_line function. To enhance error reporting of failures in tokenize_file also, the tokenize_file should return errors along with line_nums and those lines should be ignored in processing the parse_hba_line function. To do that, the tokenize_file should return whenever it encounters above those two errors only in pg_hba_rules case, but not for normal scenario. Is it fine to proceed with the changes? Regards, Hari Babu Fujitsu Australia
