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

From: Greg Stark <stark(at)mit(dot)edu>
To: Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>
Cc: Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, "Prabakaran, Vaishnavi" <vaishnavip(at)fast(dot)au(dot)fujitsu(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Jaime Casanova <jaime(at)2ndquadrant(dot)com>
Subject: Re: Providing catalog view to pg_hba.conf file - Patch submission
Date: 2015-03-03 18:57:36
Message-ID: CAM-w4HM_s77AdoXq3Xdu7sYOd-SpRr8E6Em_tuV4hH5=4E14tQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 3, 2015 at 6:05 PM, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)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

Attachment Content-Type Size
Catalog_view_to_HBA_settings_patch_V6S2.patch text/x-patch 19.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Corey Huinker 2015-03-03 18:58:42 Re: Patch: raise default for max_wal_segments to 1GB
Previous Message Josh Berkus 2015-03-03 18:42:45 Re: Normalizing units for postgresql.conf WAS: Patch: raise default for max_wal_segments to 1GB