Re: pg_hba_file_settings view patch

On Tue, Jan 24, 2017 at 6:17 PM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Mon, Jan 23, 2017 at 5:13 PM, Haribabu Kommi
> <kommi(dot)haribabu(at)gmail(dot)com> wrote:
> > On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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