Re: pg_hba_file_settings view patch

On Wed, Jan 25, 2017 at 2:50 PM, Ashutosh Bapat <
ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:

> On Wed, Jan 25, 2017 at 6:34 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
> > On Tue, Jan 24, 2017 at 11:19 PM, Ashutosh Bapat
> > <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
> >> On Mon, Jan 23, 2017 at 1:43 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:
> >>>>
> >>>> Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> writes:
> >>>> > [ pg_hba_rules_10.patch ]
> >>>>
> >>>> I took a quick look over this.
> >>>
> >>>
> >>> Thanks for the review.
> >>>
> >>>>
> >>>> * 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.
> >>>
> >>
> >> Would it be better, if we could parse each HBA line within
> >> PG_TRY()/PG_CATCH() and read errmsg from errordata stack in
> >> PG_CATCH()? We do that only when errcode is ERRCODE_CONFIG_FILE_ERROR,
> >> PG_THROWing otherwise. That's probably a bad idea but wanted to put it
> >> out as it came to me. It would eliminate a lot of changes in this
> >> patch.
> >
> > It still needs to save the error message string somewhere. So I am not
> > sure that it would save much patch size.
>
> My understanding is that ereport (and some other calls included in
> that statement) call saves it on errordata stack before jumping to the
> handler.

All the ereport messages of level are LOG, because of this reason, because
of this reason even if we use the TRY/CATCH, it doesn't work. As the
messages gets printed to the logfile and continue to process the next
statement.

Regards,
Hari Babu
Fujitsu Australia