From: | Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | Michael Paquier <michael(dot)paquier(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 04:28:29 |
Message-ID: | CAJrrPGfwCobyUvd7tBe=xFxzZ4uvDaXT6eHiH3Yn+=ufmgoDMg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Jim Nasby | 2017-01-25 04:43:27 | Re: Faster methods for getting SPI results (460% improvement) |
Previous Message | Ashutosh Bapat | 2017-01-25 03:50:41 | Re: pg_hba_file_settings view patch |