Hi
It looks well, I have only one objection.
I am not sure so function "hba_settings" should be in file guc.c - it has
zero relation to GUC.
Maybe hba.c file is better probably.
Other opinions?
2015-02-27 7:30 GMT+01:00 Haribabu Kommi <kommi.harib...@gmail.com>:
> On Sat, Feb 7, 2015 at 8:26 PM, Pavel Stehule <pavel.steh...@gmail.com>
> wrote:
> > Hi
> >
> > I am sending a review of this patch.
>
> Thanks for the review. sorry for the delay.
>
> > 4. Regress tests
> >
> > test rules ... FAILED -- missing info about new view
>
> Thanks. Corrected.
>
> > My objections:
> >
> > 1. data type for "database" field should be array of name or array of
> text.
> >
> > When name contains a comma, then this comma is not escaped
> >
> > currently: {omega,my stupid extremly, name2,my stupid name}
> >
> > expected: {"my stupid name",omega,"my stupid extremly, name2"}
> >
> > Same issue I see in "options" field
>
> Changed including the user column also.
>
> > 2. Reload is not enough for content refresh - logout is necessary
> >
> > I understand, why it is, but it is not very friendly, and can be very
> > stressful. It should to work with last reloaded data.
>
> At present the view data is populated from the variable "parsed_hba_lines"
> which contains the already parsed lines of pg_hba.conf file.
>
> During the reload, the hba entries are reloaded only in the postmaster
> process
> and not in every backend, because of this reason the reloaded data is
> not visible until
> the session is disconnected and connected.
>
> Instead of changing the SIGHUP behavior in the backend to load the hba
> entries
> also, whenever the call is made to the view, the pg_hba.conf file is
> parsed to show
> the latest reloaded data in the view. This way it doesn't impact the
> existing behavior
> but it may impact the performance because of file load into memory every
> time.
>
your solution is ok, simply and clean. Performance for this view should not
be critical and every time will be faster than login as root and searching
pg_hba.conf
Regards
Pavel
>
> Updated patch is attached. comments?
>
> Regards,
> Hari Babu
> Fujitsu Australia
>
