Re: pg_hba_file_settings view patch

From: Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: pg_hba_file_settings view patch
Date: 2017-01-25 03:50:41
Message-ID: CAFjFpRfxxz5yDgRjs0Vfi4NwwGH8GzYdkBpsDMJ+jOkR6q6BLQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haribabu Kommi 2017-01-25 04:28:29 Re: pg_hba_file_settings view patch
Previous Message Stephen Frost 2017-01-25 03:33:33 \h tab-completion