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. >> * getauthmethod() might be better replaced with an array. And doesn't it >> produce uninitialized-variable warnings for you? > > > No, i am not getting any warnings. > Changed to a static array. Thanks. Probably we should update parse_hba_line() to keep it in sync with the array. But that may be a separate add-on patch. Rest of the patch looks good to me. -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers