On Wed, Mar 4, 2015 at 5:57 AM, Greg Stark <stark@mit.edu> wrote:
> On further review I've made a few more changes attached.
>
> I think we should change the column names to "users" and "databases"
> to be clear they're lists and also to avoid the "user" SQL reserved
> word.
>
> I removed the dependency on strlist_to_array which is in
> objectaddress.c which isn't a very sensible dependency -- it does seem
> like it would be handy to have a list-based version of construct_array
> moved to arrayfuncs.c but for now it's not much more work to handle
> these ourselves.
>
> I changed the options to accumulate one big array instead of an array
> of bunches of options. Previously you could only end up with a
> singleton array with a comma-delimited string or a two element array
> with one of those and map=.
The changes are fine, this eliminates the unnecessary dependency on
objectaddress.
> I think the error if pg_hba fails to reload needs to be LOG. It would
> be too unexpected to the user who isn't necessarily the one who issued
> the SIGHUP to spontaneously get a warning.
>
> I also removed the "mask" from local entries and made some of the
> NULLS that shouldn't be possible to happen (unknown auth method or
> connection method) actually throw errors.
changes are fine. All the tests are passed.
Out of curiosity, regarding the result materialize code addition, Any
way the caller of "hba_settings" function
"ExecMakeTableFunctionResult" also stores the results in tuple_store.
Is there any advantage
doing it in hba_settings function rather than in the caller?
Regards,
Hari Babu
Fujitsu Australia