Re: pg_hba_file_settings view patch

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.
--
Michael