On Sat, Feb 7, 2015 at 8:26 PM, Pavel Stehule <pavel.stehule@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
