On Wed, Jan 25, 2017 at 2:50 PM, Ashutosh Bapat < ashutosh.ba...@enterprisedb.com> wrote:
> On Wed, Jan 25, 2017 at 6:34 AM, Michael Paquier > <michael.paqu...@gmail.com> wrote: > > On Tue, Jan 24, 2017 at 11:19 PM, Ashutosh Bapat > > <ashutosh.ba...@enterprisedb.com> wrote: > >> On Mon, Jan 23, 2017 at 1:43 PM, Haribabu Kommi > >> <kommi.harib...@gmail.com> wrote: > >>> > >>> > >>> On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >>>> > >>>> Haribabu Kommi <kommi.harib...@gmail.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
