Re: Providing catalog view to pg_hba.conf file - Patch submission

On Tue, Mar 3, 2015 at 6:05 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:

> What about a separate column that's just the text from pg_hba? Or is that what you're opposed to?

I'm not sure what you mean by that. There's a rawline field we could
put somewhere but it contains the entire line.

> FWIW, I'd say that having the individual array elements be correct is more
> important than what the result of array_out is. That way you could always do
> array_to_string(..., ', ') and get valid pg_hba output.

Well I don't think you can get that without making the view less
useful for every other purpose.

Like, I would want to be able to do WHERE "user" @> array[?] or WHERE
database = array[?] or to join against a list of users or databases
somewhere else.

To do what you suggest would mean the tokens will need to be quoted
based on pg_hba.conf syntax requirements. That would mean I would need
to check each variable or join value against pg_hba.conf's quoting
requirements to compare with it. It seems more practical to have that
knowledge if you're actually going to generate a pg_hba.conf than to
pass around these quoted strings all the time.

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=.

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.

--
greg