On 15 August 2016 at 12:17, Haribabu Kommi <kommi.harib...@gmail.com> wrote:

> comments?

This looks like a good feature contribution, thanks.

At present the patch doesn't apply cleanly, please rebase.

The patch doesn't contain any tests, which means I can't see what the
output looks like, so I can't judge the exact usefulness of this
particular patch. ISTM likely that there will be some detailed points
to review in there somewhere.

Do we want line number, or priority order? i.e. do we want to count
comment lines or just main rule lines? I prefer latter.
Various other questions along those lines needed, once I can see the output.

What is push_jsonb_string_value() etc..?
If there is required infrastructure there is no explanation of why.
Suggest you explain and/or split into two.

pg_hba_file_settings seems a clumsy name. I'd prefer pg_hba_settings,
since that name could live longer than the concept of pg_hba.conf,
which seems likely to become part of ALTER SYSTEM in future, so we
wouldn't really want the word "file" in there.

I've not seen anything yet to make me think a commit for this wouldn't
happen once we've worked the detail.

Thanks

-- 
Simon Riggs                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to