Обсуждение: pg_hba_file_settings view patch

Hi All,

While working on pg_hba_lookup function that can be used to lookup for an client
authentication that can be matched for given input parameters, Tom raised some
concrete use case issues in the following mail [1]. In this same
thread, he raised
some advantages of having a view similar like pg_file_settings view
for pg_hba.conf
also.

Here I attached a patch that implements the pg_hba_file_settings view
that displays
all the rows in pg_hba.conf. In case if any error exists in the
authentication rule, the
corresponding error is displayed similar like pg_file_settings.

This view can be used to verify whether there exists any problems or
not in the pg_hba.conf
before it reloads into the system. This view cannot be used to check
similar like
pg_hba_lookup function to find out which rule maps to the
corresponding input connection.

comments?

[1] - https://www.postgresql.org/message-id/28434.1468246200%40sss.pgh.pa.us

Regards,
Hari Babu
Fujitsu Australia

On 15 August 2016 at 12:17, Haribabu Kommi <kommi.haribabu@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

Re: Simon Riggs 2016-09-03 <CANP8+jLFDeCJ7YWuzSodKXD85oNy2Kxa40U5GRnry6ms9Mz+5A@mail.gmail.com>
> 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.

IMHO "settings" is wrong here. "pg_file_settings" means "settings in
config file (that might not been applied yet)". The contents of
pg_hba.conf are not config settings, but there doesn't appear to be a
standard name for them - 19.1 calls them "records".

Given that the patch seems to report what's on disk, "pg_hba_file"
seems a good name to me. Even if ALTER SYSTEM should become able to
update the file, it'd still be a file. (If it were the actual running
config, I'd go for "pg_hba_rules".)

Christoph

On Mon, Sep 5, 2016 at 4:09 PM, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
> On Sun, Sep 4, 2016 at 1:44 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> On 15 August 2016 at 12:17, Haribabu Kommi <kommi.haribabu@gmail.com>
>> wrote:
>>
>> > comments?
>>
>> This looks like a good feature contribution, thanks.
>>
>> At present the patch doesn't apply cleanly, please rebase.
>
>
> Rebased patch is attached.

Moved to next CF as there is a patch and no reviews.

+       push_jsonb_string_key(&parseState, "map");
+       push_jsonb_string_value(&parseState, hba->usermap);
[...]
+    <row>
+     <entry><structfield>options</structfield></entry>
+     <entry><type>jsonb</type></entry>
+     <entry>Configuration options set for authentication method</entry>
+    </row>
Why is it an advantage to use jsonb here instead of a simple array
made of name=value? If they were nested I'd see a case for it but it
seems to me that as presented this is just an overkill. In short, I
think that this patch needs a bit of rework, so I am marking it as
returned with feedback.
-- 
Michael

On 10/2/16, Michael Paquier <michael.paquier@gmail.com> wrote:
> +       push_jsonb_string_key(&parseState, "map");
> +       push_jsonb_string_value(&parseState, hba->usermap);
> [...]
> +    <row>
> +     <entry><structfield>options</structfield></entry>
> +     <entry><type>jsonb</type></entry>
> +     <entry>Configuration options set for authentication method</entry>
> +    </row>
> Why is it an advantage to use jsonb here instead of a simple array
> made of name=value? If they were nested I'd see a case for it but it
> seems to me that as presented this is just an overkill.

I guess for ability to use filtering like:

SELECT * FROM pg_hba_rules WHERE options->>radiusserver LIKE '%.example.com';

I think it would be harder if options is an array of strings...

-- 
Best regards,
Vitaly Burovoy

On Mon, Oct 3, 2016 at 3:25 PM, Vitaly Burovoy <vitaly.burovoy@gmail.com> wrote:
> I guess for ability to use filtering like:
>
> SELECT * FROM pg_hba_rules WHERE options->>radiusserver LIKE '%.example.com';
>
> I think it would be harder if options is an array of strings...

With unnest() and a matching pattern, not that hard but..
-- 
Michael

On 10/2/16, Michael Paquier <michael.paquier@gmail.com> wrote:
> On Mon, Oct 3, 2016 at 3:25 PM, Vitaly Burovoy <vitaly.burovoy@gmail.com>
> wrote:
>> I guess for ability to use filtering like:
>>
>> SELECT * FROM pg_hba_rules WHERE options->>radiusserver LIKE
>> '%.example.com';
>>
>> I think it would be harder if options is an array of strings...
>
> With unnest() and a matching pattern, not that hard but..

I'm not a user of that feature, and I don't know how pg_hba files look
like in really big companies...

But for me filtering is more complicated than just a single comparison.
What about more complex filtering --- several radiusserver and a user(s):

WHERE   options->>radiusserver = ANY(ARRAY['a.example.com', 'g.example.com'])   AND   options->>radiusidentifier =
ANY(ARRAY['ID_a','ID_b', 'ID_c',
 
'ID_d', 'ID_e'])  -- or even a subquery

Again, I don't know whether it will be widely used, but in case of
multiple param_name->param_value settings (column "options") I'd like
to see native key-value store rather than array of strings (according
to POLA).

I guess you're expecting "key=value" format as they are written in the
pg_hba file (and described in the doc), but sometimes they can be
parsed and output differs from exact pg_hba records (for instance look
at "ldapurl" parameter).

-- 
Best regards,
Vitaly Burovoy

Haribabu Kommi wrote:
> On Mon, Oct 3, 2016 at 3:51 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:

> Yes, I agree that adding these JSONB utility functions for this view
> is an overkill, but I thought that these are may be useful for some
> users if it is a JSONB type instead of array.

Peter Eisentraut said he'd like JSON better:
https://www.postgresql.org/message-id/5547DB0A.2020904@gmx.net
I asked twice about the use of JSON, suggesting an array instead:
https://www.postgresql.org/message-id/20151204163147.GZ2763@alvherre.pgsql
https://www.postgresql.org/message-id/20160201215714.GA98800@alvherre.pgsql

I now think that we should figure out what it is that we want before we
continue to request it to be changed over and over.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Oct 25, 2016 at 3:23 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Haribabu Kommi wrote:
>> On Mon, Oct 3, 2016 at 3:51 PM, Michael Paquier <michael.paquier@gmail.com>
>> wrote:
>
>> Yes, I agree that adding these JSONB utility functions for this view
>> is an overkill, but I thought that these are may be useful for some
>> users if it is a JSONB type instead of array.
>
> Peter Eisentraut said he'd like JSON better:
> https://www.postgresql.org/message-id/5547DB0A.2020904@gmx.net
> I asked twice about the use of JSON, suggesting an array instead:
> https://www.postgresql.org/message-id/20151204163147.GZ2763@alvherre.pgsql
> https://www.postgresql.org/message-id/20160201215714.GA98800@alvherre.pgsql
>
> I now think that we should figure out what it is that we want before we
> continue to request it to be changed over and over.

That sounds like a good idea.  :-)

FWIW, I'm -1 on using JSON here.  I don't believe that we should start
using JSON all over the place just because we can.  If we do that,
we'll end up with a mishmash of styles, and maybe look silly when JSON
is replaced by the new and much better SDGJHSDR format.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> FWIW, I'm -1 on using JSON here.  I don't believe that we should start
> using JSON all over the place just because we can.  If we do that,
> we'll end up with a mishmash of styles, and maybe look silly when JSON
> is replaced by the new and much better SDGJHSDR format.

I concur.  JSON isn't a core datatype and I don't want to see it treated
as one.  We should redesign this view so that it doesn't rely on anything
more advanced than arrays.
        regards, tom lane

On 10/26/2016 12:24 PM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> FWIW, I'm -1 on using JSON here.  I don't believe that we should start
>> using JSON all over the place just because we can.  If we do that,
>> we'll end up with a mishmash of styles, and maybe look silly when JSON
>> is replaced by the new and much better SDGJHSDR format.
> 
> I concur.  JSON isn't a core datatype and I don't want to see it treated
> as one.  We should redesign this view so that it doesn't rely on anything
> more advanced than arrays.

Huh?  Sure it is.   Ships in PostgreSQL-core.

I mean, I'm not particularly in favor of using JSON for this (arrays
seem OK), but that seems like an invalid reason not to.

-- 
--
Josh Berkus
Red Hat OSAS
(any opinions are my own)

On 10/26/2016 12:54 PM, Josh Berkus wrote:
> On 10/26/2016 12:24 PM, Tom Lane wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> FWIW, I'm -1 on using JSON here.  I don't believe that we should start
>>> using JSON all over the place just because we can.  If we do that,
>>> we'll end up with a mishmash of styles, and maybe look silly when JSON
>>> is replaced by the new and much better SDGJHSDR format.
>>
>> I concur.  JSON isn't a core datatype and I don't want to see it treated
>> as one.  We should redesign this view so that it doesn't rely on anything
>> more advanced than arrays.
>
> Huh?  Sure it is.   Ships in PostgreSQL-core.
>
> I mean, I'm not particularly in favor of using JSON for this (arrays
> seem OK), but that seems like an invalid reason not to.

-1 to JSON for this.

JD

>


-- 
Command Prompt, Inc.                  http://the.postgres.company/                        +1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.

Josh Berkus <josh@agliodbs.com> writes:
> On 10/26/2016 12:24 PM, Tom Lane wrote:
>> I concur.  JSON isn't a core datatype and I don't want to see it treated
>> as one.  We should redesign this view so that it doesn't rely on anything
>> more advanced than arrays.

> Huh?  Sure it is.   Ships in PostgreSQL-core.

To my way of thinking it's a nonstandard extension.  The fact that we
chose to package it in core and not as an extension doesn't alter the
fact that it's peripheral to the system and nothing else depends on it.
I'd like to keep things that way.  I wouldn't want any core-system
functionality to start depending on the geometric types, either.
        regards, tom lane

On Thu, Oct 27, 2016 at 5:11 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Josh Berkus <josh@agliodbs.com> writes:
>> On 10/26/2016 12:24 PM, Tom Lane wrote:
>>> I concur.  JSON isn't a core datatype and I don't want to see it treated
>>> as one.  We should redesign this view so that it doesn't rely on anything
>>> more advanced than arrays.
>
>> Huh?  Sure it is.   Ships in PostgreSQL-core.
>
> To my way of thinking it's a nonstandard extension.  The fact that we
> chose to package it in core and not as an extension doesn't alter the
> fact that it's peripheral to the system and nothing else depends on it.
> I'd like to keep things that way.  I wouldn't want any core-system
> functionality to start depending on the geometric types, either.

I got a similar opinion regarding this patch to be honest after
looking at it, seeing actually with a bad eye the use of fancy data
types that are not well-spread among the other catalog views and
functions. So -1 for JSON and +1 for arrays.
-- 
Michael

On Wed, Oct 26, 2016 at 11:04 PM, Joshua D. Drake <jd@commandprompt.com> wrote:
> On 10/26/2016 12:54 PM, Josh Berkus wrote:
>> I mean, I'm not particularly in favor of using JSON for this (arrays
>> seem OK), but that seems like an invalid reason not to.
>
> -1 to JSON for this.

Sigh. Well I tried to review this patch in a previous iteration so let
me give some context.

The fundamental problem is that the pga_hba.conf file has some bits of
complex structure that aren't easily captured by linear arrays. The
problem I struggled with most was the keywords like "all", "samerole",
and "replication". A simple array of text makes it awkward to
distinguish those keywords from the quoted text values with the same
content. And then there are the ldap options which naturally would be
a data type like json or htab.

Some people wanted to store strings like '"all"' with the quotes which
I thought was ugly and functionally less useful because it would be
hard to query and impossible to join against things like pg_users.
Others wanted to give up the idea of expanding the entries at all and
just have a single string for the whole line which I thought was
pointless -- you may as well just read the file then.

Personally my recommendation was to ignore the problem. Just have
arrays of text and document that if you have a real user by the name
"all" or "samerole" then the view cannot be interpreted accurately.
Tools like pgadmin which want to use the view could check for such
users and display a warning or error rather than inaccurate
information.

If there's any support for my recommendation I'm still happy to pick
up the patch again and commit it.

-- 
greg

Greg Stark wrote:

> The fundamental problem is that the pga_hba.conf file has some bits of
> complex structure that aren't easily captured by linear arrays. The
> problem I struggled with most was the keywords like "all", "samerole",
> and "replication". A simple array of text makes it awkward to
> distinguish those keywords from the quoted text values with the same
> content. And then there are the ldap options which naturally would be
> a data type like json or htab.

Hmm I thought we had decided that such keywords would live in separate
arrays, i.e. you have one array for plain names and another array for
keyword stuff.  Then it's not ambiguous anymore.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Nov 7, 2016 at 12:36 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> The added regression test fails for the cases where the server is loaded
> with
> different pg_hba.conf rules during installcheck verification. Updated patch
> is
> attached with removing those tests.

That's not a full review as I just glanced at this patch a couple of seconds...
#include "utils/guc.h"
+#include "utils/jsonb.h"#include "utils/lsyscache.h"
You don't need to include this header when using arrays.

Implementing a test case is possible as well using the TAP
infrastructure. You may want to look at it and help folks testing the
patch more easily with a set of configurations in pg_hba.conf that
cover a maximum of code paths in your patch.
-- 
Michael

make check run with this patch shows server crashes. regression.out
attached. I have run make check after a clean build, tried building it
after running configure, but the problem is always reproducible. Do
you see this problem?

Also the patch has a white space error.
git diff --check
src/backend/utils/init/postinit.c:729: space before tab in indent.
+       /*

On Thu, Nov 10, 2016 at 11:40 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Mon, Nov 7, 2016 at 3:36 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> On Mon, Nov 7, 2016 at 12:36 PM, Haribabu Kommi
>> <kommi.haribabu@gmail.com> wrote:
>> > The added regression test fails for the cases where the server is loaded
>> > with
>> > different pg_hba.conf rules during installcheck verification. Updated
>> > patch
>> > is
>> > attached with removing those tests.
>>
>> That's not a full review as I just glanced at this patch a couple of
>> seconds...
>>
>>  #include "utils/guc.h"
>> +#include "utils/jsonb.h"
>>  #include "utils/lsyscache.h"
>> You don't need to include this header when using arrays.
>
>
> Thanks for the review. Fixed in the updated patch with
> additional error messages are also added.
>
>>
>> Implementing a test case is possible as well using the TAP
>> infrastructure. You may want to look at it and help folks testing the
>> patch more easily with a set of configurations in pg_hba.conf that
>> cover a maximum of code paths in your patch.
>
>
> Added a tap test under src/test folder to cover maximum code changes.
>
> Regards,
> Hari Babu
> Fujitsu Australia
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Wed, Nov 16, 2016 at 4:40 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> make check run with this patch shows server crashes. regression.out
> attached. I have run make check after a clean build, tried building it
> after running configure, but the problem is always reproducible. Do
> you see this problem?
>
> Also the patch has a white space error.
> git diff --check
> src/backend/utils/init/postinit.c:729: space before tab in indent.
> +       /*
>
I looked at the patch in some more details and here are some more comments
1. In catalog.sgml, the sentence "If the configuration file contains any errors
..." looks redundant, as description of "error" field says so. Removed it in
the attached patch. In that example, you might want to provide pg_hba.conf
contents to help understand the view output.

2. I think the view will be useful, if loading the file did not have the
desired effects, whether because of SIGHUP or a fresh start. So, may be the
sentence "for diagnosing problems if a SIGHUP signal did not have the desired
effects.", should be changed to be more generic e.g. ... if loading file did
not have ... .

3. Something wrong with the indentation, at least not how pg_indent would indent
the variable names.
+typedef struct LookupHbaLineCxt
+{
+    MemoryContext memcxt;
+    TupleDesc    tupdesc;
+    Tuplestorestate *tuple_store;
+} LookupHbaLineCxt;

+static void lookup_hba_line_callback(void *context, int lineno,
HbaLine *hba, const char *err_msg);
Overflows 80 character limit.

in parse_hba_line()
-        ereport(LOG,
+        ereport(level,                (errcode(ERRCODE_CONFIG_FILE_ERROR),                 errmsg("invalid connection
type\"%s\"",                        token->string),                 errcontext("line %d of configuration file \"%s\"",
                         line_num, HbaFileName)));
 
+        *err_msg = pstrdup(_("invalid connection type"));

Is the difference between error reported to error log and that in the view
intentional? That brings to another question. Everywhere, in similar code, the
patch adds a line *err_msg = pstrdup() or psprinf() and copies the arguements
to errmsg(). Someone modifying the error message has to duplicate the changes.
Since the code is just few lines away, it may not be hard to duplicate the
changes, but still that's a maintenance burder. Isn't there a way to compute
the message once and use it twice? show_all_file_settings() used for
pg_file_settings also has similar problem, so may be it's an accepted practice.
There are multiple instances of such a difference, but may be the invalid value
can be found out from the value of the referenced field (which will be part of
the view). So, may be it's ok. But that not true with the difference below.
gai_strerror() may not be obvious from the referenced field.
-                ereport(LOG,
+                ereport(level,                        (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("invalidIP address \"%s\": %s",                                str, gai_strerror(ret)),
errcontext("line%d of configuration file \"%s\"",                                    line_num, HbaFileName)));
     if (gai_result)                    pg_freeaddrinfo_all(hints.ai_family, gai_result);
 
+                *err_msg = pstrdup(_("invalid IP address"));

4.
+    if (!rsi || !IsA(rsi, ReturnSetInfo) ||
+        (rsi->allowedModes & SFRM_Materialize) == 0)
+        ereport(ERROR,
+                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                 errmsg("set-valued function called in context that
cannot accept a set")));
show_all_file_settings(), a function similar to this one, splits the condition
above into two and throws different error message for each of them.   /* Check to see if caller supports us returning a
tuplestore*/   if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))       ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),               errmsg("set-valued function called in context that
 
cannot accept a set")));   if (!(rsinfo->allowedModes & SFRM_Materialize))       ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),               errmsg("materialize mode required, but it is not " \
         "allowed in this context")));
 
Why is this difference?

5. May be you want to rename "type" attribute to "connection_type" to be
explicit.

6. The attribute names "keyword_database" and "keyword_user" do not seem to be
appropriate. They do not look like keywords as such. They are more like
synonyms or collection (value replication is an exception). May be you want to
rename those as "database_keyword" or "user_keyword" similar to the naming
convention of token_is_a_database_keyword(). I agree that the usage
can not be described in a single phrase correctly, and pg_hba.conf
documentation too doesn't help much. Similarly for keyword_address.

7. Also, each of the fields, database, user, address can contain multiple
values in pg_hba.conf. So may be corresponding attributes should be named as
plural rather than singular.

8. If any of the parsed lines has an error parse_hba_line() returns a NULL
line. In order to avoid memory leak, load_hba() runs this function in a memory
context, which is destroyed when an error is encountered. This also destroys
any previous lines which were parsed correctly. IIUC, the patch uses the
callback to save the contents of those lines in a different context, so that an
error wouldn't destroy those. But using a callback doesn't seem like a good way
to do this. Furthermore the code assumes that if callback is provided the error
level is always DEBUG3 or the caller doesn't want to update the saved
authentication rules etc. If in future someone wants to add another callback
function but doesn't want error level to be DEBUG3 or still wants to update the
saved rules, we will need conditional statements based on the value of
callback. That doesn't seems to be something which should be done with
callbacks. I don't think that's flexible. A better design may be to let
load_hba() accept errorlevel, and flag indicating whether to ignore errors as
an argument and return a list of parsed lines. If there is an error and the
flag indicates not to ignore the error, we destroy the memory context and
return NIL. The list can be then used to update the saved hba rules or to
process further (e.g. to feed the function hba_rules()). hbacxt also can an
OUTPUT arguemnt to the function or an argument passed in by the caller.

9. I am not able to understand, why does this patch need changes to
load_ident(). Sorry, if I have missed any previous discussion on this topic.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Fri, Nov 18, 2016 at 12:23 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Thu, Nov 17, 2016 at 10:13 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>>
>> On Wed, Nov 16, 2016 at 4:40 PM, Ashutosh Bapat
>> <ashutosh.bapat@enterprisedb.com> wrote:
>> > make check run with this patch shows server crashes. regression.out
>> > attached. I have run make check after a clean build, tried building it
>> > after running configure, but the problem is always reproducible. Do
>> > you see this problem?
>
>
> Thanks for reviewing the patch.
>
> No. I am not able to reproduce this problem.
> make check works fine in my system.

It could be because of some un-initialised variable, which is
initialized appropriately by default on your machine but not on my
machine. I first applied your pg_hba_rules... patch, ran regression.
It didn't crash. then I applied patch for discard_hba... and it
started crashing. Does that give you any clue? Here's regression.out
file for make installcheck. Here is error log snippet that shows a
SIGSEGV there.
2016-11-22 15:47:11.939 IST [86206] LOG:  worker process: parallel
worker for PID 86779 (PID 86780) was terminated by signal 11:
Segmentation fault
2016-11-22 15:47:11.939 IST [86206] LOG:  terminating any other active
server processes
2016-11-22 15:47:11.939 IST [86779] WARNING:  terminating connection
because of crash of another server process
2016-11-22 15:47:11.939 IST [86779] DETAIL:  The postmaster has
commanded this server process to roll back the current transaction and
exit, because another server process exited abnormally and possibly
corrupted shared memory.

Applying those patches in any order doesn't matter.

>
> From the regression.out file, the crash occurred in select_parallel.out,
> I don't think this patch has any affect on that test.

The changes in postinit.c may have that impact. Just a guess though. I
haven't debugged the crash myself.


>>
>> I looked at the patch in some more details and here are some more comments
>> 1. In catalog.sgml, the sentence "If the configuration file contains any
>> errors
>> ..." looks redundant, as description of "error" field says so. Removed it
>> in
>> the attached patch. In that example, you might want to provide pg_hba.conf
>> contents to help understand the view output.
>
>
> updated details, but not exactly what you said. check it once.
>



>
>>
>> in parse_hba_line()
>> -        ereport(LOG,
>> +        ereport(level,
>>                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
>>                   errmsg("invalid connection type \"%s\"",
>>                          token->string),
>>                   errcontext("line %d of configuration file \"%s\"",
>>                              line_num, HbaFileName)));
>> +        *err_msg = pstrdup(_("invalid connection type"));
>>
>> Is the difference between error reported to error log and that in the view
>> intentional? That brings to another question. Everywhere, in similar code,
>> the
>> patch adds a line *err_msg = pstrdup() or psprinf() and copies the
>> arguements
>> to errmsg(). Someone modifying the error message has to duplicate the
>> changes.
>> Since the code is just few lines away, it may not be hard to duplicate the
>> changes, but still that's a maintenance burder. Isn't there a way to
>> compute
>> the message once and use it twice? show_all_file_settings() used for
>> pg_file_settings also has similar problem, so may be it's an accepted
>> practice.
>> There are multiple instances of such a difference, but may be the invalid
>> value
>> can be found out from the value of the referenced field (which will be
>> part of
>> the view). So, may be it's ok. But that not true with the difference
>> below.
>> gai_strerror() may not be obvious from the referenced field.
>> -                ereport(LOG,
>> +                ereport(level,
>>                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
>>                           errmsg("invalid IP address \"%s\": %s",
>>                                  str, gai_strerror(ret)),
>>                           errcontext("line %d of configuration file
>> \"%s\"",
>>                                      line_num, HbaFileName)));
>>                  if (gai_result)
>>                      pg_freeaddrinfo_all(hints.ai_family, gai_result);
>> +                *err_msg = pstrdup(_("invalid IP address"));
>
>
> Reused the error string once, as in this patch it chances in many places
> compared
> to pg_file_settings, so I tend to reuse it.

Thanks. Although the new change might affect the way we translate the
messages in other languages. I am not sure. So, I will leave it for
someone with more knowledge to review.


>
>>
>> 5. May be you want to rename "type" attribute to "connection_type" to be
>> explicit.
>
>
> "type" is the keyword that is mentioned in the pg_hba.conf, I feel it is
> better
> if this view is in sync with that. If others feel the same, I can change.

Ok.


>
>>
>> 7. Also, each of the fields, database, user, address can contain multiple
>> values in pg_hba.conf. So may be corresponding attributes should be named
>> as
>> plural rather than singular.
>
>
> Same answer as to the question - 5.

Ok.

>
>>
>> 8. If any of the parsed lines has an error parse_hba_line() returns a NULL
>> line. In order to avoid memory leak, load_hba() runs this function in a
>> memory
>> context, which is destroyed when an error is encountered. This also
>> destroys
>> any previous lines which were parsed correctly. IIUC, the patch uses the
>> callback to save the contents of those lines in a different context, so
>> that an
>> error wouldn't destroy those. But using a callback doesn't seem like a
>> good way
>> to do this. Furthermore the code assumes that if callback is provided the
>> error
>> level is always DEBUG3 or the caller doesn't want to update the saved
>> authentication rules etc. If in future someone wants to add another
>> callback
>> function but doesn't want error level to be DEBUG3 or still wants to
>> update the
>> saved rules, we will need conditional statements based on the value of
>> callback. That doesn't seems to be something which should be done with
>> callbacks. I don't think that's flexible. A better design may be to let
>> load_hba() accept errorlevel, and flag indicating whether to ignore errors
>> as
>> an argument and return a list of parsed lines. If there is an error and
>> the
>> flag indicates not to ignore the error, we destroy the memory context and
>> return NIL. The list can be then used to update the saved hba rules or to
>> process further (e.g. to feed the function hba_rules()). hbacxt also can
>> an
>> OUTPUT arguemnt to the function or an argument passed in by the caller.
>
>
> hba_rules() function cannot operate on final parsed hba lines, because it
> has
> to store the error that is present in the line, that can be obtained only
> during
> the parse stage.
>
> The hba rules are needed only for the authentication purpose and those
> shouldn't
> be stored in the individual backend. Because of this reason after every
> operation
> the parsed rules are cleared.
>
> Added a flag to pass the log level.

    /*
+    * If callback function is available, then don't update the saved
+    * authentication rules.
+    */
+   if (callback)
+   {
+       MemoryContextDelete(linecxt);
+       MemoryContextSwitchTo(oldcxt);
+       MemoryContextDelete(hbacxt);
+       return true;
+   }

this still remains problematic, in case another user of load_hba wants
to pass a callback but wants to update the saved rules. Usually,
callbacks are used when the decision to modify certain logic is far
away in time and code from the actual place where the changes are to
be applied, e.g. FDW callbacks. But here we that's not the case. Is
there any precedence in code for something like this.

What we may want to do, is separate the logic of reading the hba rules
in a list and the logic to update existing rules in two different
functions e.g read_hba() and load_hba(). hba_rules() can use
read_hba() with ignore errors flag to get the list of hba lines. It
can then use this list to create tuples to be returned in hba_rules().
load_hba() will read_hba() with memory reset on error flag (same
boolean) to read the list of hba lines and update the saved rules if
there's no error. err_msg can be either a field in HbaLine, which will
be used only by hba_rules() OR read_hba() could return list of
err_msgs as a pass by ref argument.

>
>>
>> 9. I am not able to understand, why does this patch need changes to
>> load_ident(). Sorry, if I have missed any previous discussion on this
>> topic.
>
>
> Earlier, the pg_hba.conf is loaded into the Postmastercontext, this is
> causing
> problems for this patch. So In the patch it is changed into
> currentmemorycontext
> and deleted the data at the end. The similar change is carried out for ident
> functionality, because of this reason, it is shown in this patch. May be I
> can
> separate that into an individual patch.

I think we need to include the hba related changes in this patch and
indent related changes should be moved to the other patch.


--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Here's backtrace and some debugging information
Program terminated with signal 11, Segmentation fault.
#0  0x00000000007f96cd in shm_mq_sendv (mqh=0x121e998,
iov=0x7ffc9b7b79f0, iovcnt=2, nowait=1 '\001') at shm_mq.c:357
357        Assert(mq->mq_sender == MyProc);
(gdb) where
#0  0x00000000007f96cd in shm_mq_sendv (mqh=0x121e998,
iov=0x7ffc9b7b79f0, iovcnt=2, nowait=1 '\001') at shm_mq.c:357
#1  0x00000000006d8387 in mq_putmessage (msgtype=88 'X', s=0x0, len=0)
at pqmq.c:165
#2  0x0000000000515147 in ParallelWorkerMain (main_arg=141900502) at
parallel.c:1120
#3  0x0000000000783063 in StartBackgroundWorker () at bgworker.c:726
#4  0x0000000000795b77 in do_start_bgworker (rw=0x1216f00) at postmaster.c:5535
#5  0x0000000000795e4f in maybe_start_bgworker () at postmaster.c:5710
#6  0x0000000000794eb3 in sigusr1_handler (postgres_signal_arg=10) at
postmaster.c:4959
#7  <signal handler called>
#8  0x00002b005933a693 in select () from /lib/x86_64-linux-gnu/libc.so.6
#9  0x0000000000790720 in ServerLoop () at postmaster.c:1665
#10 0x000000000078fe76 in PostmasterMain (argc=8, argv=0x11eef40) at
postmaster.c:1309
#11 0x00000000006d8f1d in main (argc=8, argv=0x11eef40) at main.c:228
(gdb) p mq->mq_sender
Cannot access memory at address 0x6b636568635f707d
(gdb) p mq
$1 = (shm_mq *) 0x6b636568635f706d

Looking at this, I am wondering, how could that happen with your
patches. But every time I have tried to apply your patches and run
regression, I get this crash. Just now I tried the patches on a all
new repository and reproduced the crash.

On Tue, Nov 29, 2016 at 3:10 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Tue, Nov 22, 2016 at 9:46 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>>
>>
>> It could be because of some un-initialised variable, which is
>> initialized appropriately by default on your machine but not on my
>> machine. I first applied your pg_hba_rules... patch, ran regression.
>> It didn't crash. then I applied patch for discard_hba... and it
>> started crashing. Does that give you any clue? Here's regression.out
>> file for make installcheck. Here is error log snippet that shows a
>> SIGSEGV there.
>> 2016-11-22 15:47:11.939 IST [86206] LOG:  worker process: parallel
>> worker for PID 86779 (PID 86780) was terminated by signal 11:
>> Segmentation fault
>> 2016-11-22 15:47:11.939 IST [86206] LOG:  terminating any other active
>> server processes
>> 2016-11-22 15:47:11.939 IST [86779] WARNING:  terminating connection
>> because of crash of another server process
>> 2016-11-22 15:47:11.939 IST [86779] DETAIL:  The postmaster has
>> commanded this server process to roll back the current transaction and
>> exit, because another server process exited abnormally and possibly
>> corrupted shared memory.
>>
>> Applying those patches in any order doesn't matter.
>
>
> I am not able to reproduce the crash both in debug and release mode
> builds with both check and installcheck options.
>
> Can you please share the back trace of the crash, so that it will be helpful
> for me to locate the problem.
>
>
> Regards,
> Hari Babu
> Fujitsu Australia



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On 4 January 2017 at 03:54, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:

> Latest patch is attached.

The "method" column should be called "auth" or "auth_method" or "authentication"

I think we should have some tests, but I'll hear your views on that.
Perhaps we can include a test/sample pg_hba.conf for use in tests.

Since we've had crashes, I suggest running the test 10000 times and
checks for leaks and crashes.

If its safe we can move towards commit. Thanks

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

On Thu, Jan 5, 2017 at 5:10 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On 4 January 2017 at 03:54, Haribabu Kommi <kommi.haribabu@gmail.com> wrote:
>
>> Latest patch is attached.
>
> The "method" column should be called "auth" or "auth_method" or "authentication"
>
> I think we should have some tests, but I'll hear your views on that.
> Perhaps we can include a test/sample pg_hba.conf for use in tests.
>
> Since we've had crashes, I suggest running the test 10000 times and
> checks for leaks and crashes.
>
> If its safe we can move towards commit. Thanks

Could you hold on a bit to commit that? I'd like to look at it in more
details. At quick glance, there is for example no need to use
CreateTemplateTupleDesc and list the columns both in pg_proc.h and the
C routine itself. And memset() can be used in fill_hba_line for the
error code path.
-- 
Michael

On Thu, Jan 5, 2017 at 1:58 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Could you hold on a bit to commit that? I'd like to look at it in more
> details. At quick glance, there is for example no need to use
> CreateTemplateTupleDesc and list the columns both in pg_proc.h and the
> C routine itself. And memset() can be used in fill_hba_line for the
> error code path.

And here we go.

+<programlisting>
+postgres=# select * from pg_hba_rules;
[... large example ...]
+
+</programlisting>
It would be nice to reduce the width of this example. That's not going
to be friendly with the generated html.

+       switch (hba->conntype)
+       {
+           case ctLocal:
+               values[index] = CStringGetTextDatum("local");
+               break;
+           case ctHost:
+               values[index] = CStringGetTextDatum("host");
+               break;
+           case ctHostSSL:
+               values[index] = CStringGetTextDatum("hostssl");
+               break;
+           case ctHostNoSSL:
+               values[index] = CStringGetTextDatum("hostnossl");
+               break;
+           default:
+               elog(ERROR, "unexpected connection type in parsed HBA entry");
+               break;
+       }
Here let's remove the break clause and let compilers catch problem
when they show up.

+   if (hba->pamservice)
+   {
+       initStringInfo(&str);
+       appendStringInfoString(&str, "pamservice=");
+       appendStringInfoString(&str, hba->pamservice);
+       options[noptions++] = CStringGetTextDatum(str.data);
+   }
There is a bunch of code duplication here. I think that it would make
more sense to encapsulate that into a routine, at least let's use
appendStringInfo and let's group the two calls together.

+/* LDAP supports 10 currently, keep this well above the most any
method needs */
+#define MAX_OPTIONS 12
Er, why? There is an assert already, that should be enough.

=# \d pg_hba_rules              View "pg_catalog.pg_hba_rules"     Column      |  Type   | Collation | Nullable |
Default
------------------+---------+-----------+----------+---------line_number      | integer |           |          |type
        | text    |           |          |keyword_database | text[]  |           |          |database         | text[]
|          |          |keyword_user     | text[]  |           |          |user_name        | text[]  |           |
   |keyword_address  | text    |           |          |address          | inet    |           |          |netmask
  | inet    |           |          |hostname         | text    |           |          |method           | text    |
     |          |options          | text[]  |           |          |error            | text    |           |
|
keyword_database and database map actually to the same thing if you
refer to a raw pg_hba.conf file because they have the same meaning for
user. You could simplify the view and just remove keyword_database,
keyword_user and keyword_address. This would simplify your patch as
well with all hte mumbo-jumbo to see if a string is a dedicated
keyword or not. In its most simple shape pg_hba_rules should show to
the user as an exact map of the entries of the raw file.

I have copied the example file of pg_hba.conf, reloaded it:
https://www.postgresql.org/docs/devel/static/auth-pg-hba-conf.html
And then the output result gets corrupted by showing up free()'d results:
null   | null    | \x7F\x7F\x7F\x7F\x7F

+   if (err_msg)
+   {
+       /* type */
+       index++;
+       nulls[index] = true;
[... long sequence ...]
Please let's use MemSet here and remove this large chunk of code...

+   if (!superuser())
+       ereport(ERROR,
+               (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                (errmsg("must be superuser to view pg_hba.conf settings"))));
Access to the function is already revoked, so what's the point of this
superuser check?

+   tupdesc = CreateTemplateTupleDesc(NUM_PG_HBA_LOOKUP_ATTS, false);
+   TupleDescInitEntry(tupdesc, (AttrNumber) 1, "line_number",
+                      INT4OID, -1, 0);
There is no need to list all the columns here. You can just use
get_call_result_type() and be done with it as the types and columns
names are already listed in the pg_proc entry of the new function.
-- 
Michael

On Tue, Jan 17, 2017 at 10:19 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> On Tue, Jan 10, 2017 at 6:35 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>> +/* LDAP supports 10 currently, keep this well above the most any
>> method needs */
>> +#define MAX_OPTIONS 12
>> Er, why? There is an assert already, that should be enough.
>
>
> Which Assert? This macro is used to verify that the maximum number
> of authentication options that are possible for a single hba line.

That one:
+   Assert(noptions <= MAX_OPTIONS);
+   if (noptions)
+       return PointerGetDatum(
+               construct_array(options, noptions, TEXTOID, -1, false, 'i'));

>> =# \d pg_hba_rules
>>                View "pg_catalog.pg_hba_rules"
>>       Column      |  Type   | Collation | Nullable | Default
>> ------------------+---------+-----------+----------+---------
>>  line_number      | integer |           |          |
>>  type             | text    |           |          |
>>  keyword_database | text[]  |           |          |
>>  database         | text[]  |           |          |
>>  keyword_user     | text[]  |           |          |
>>  user_name        | text[]  |           |          |
>>  keyword_address  | text    |           |          |
>>  address          | inet    |           |          |
>>  netmask          | inet    |           |          |
>>  hostname         | text    |           |          |
>>  method           | text    |           |          |
>>  options          | text[]  |           |          |
>>  error            | text    |           |          |
>> keyword_database and database map actually to the same thing if you
>> refer to a raw pg_hba.conf file because they have the same meaning for
>> user. You could simplify the view and just remove keyword_database,
>> keyword_user and keyword_address. This would simplify your patch as
>> well with all hte mumbo-jumbo to see if a string is a dedicated
>> keyword or not. In its most simple shape pg_hba_rules should show to
>> the user as an exact map of the entries of the raw file.
>
> I removed keyword_database and keyword_user columns where the data
> in those columns can easily represent with the database and user columns.
> But for address filed can contains keywords such as "same host" and etc and
> also a hostname also. Because of this reason, this field is converted into
> 3 columns in the view.

Hm. We could as well consider cidr and use just one column. But still,
the use of inet as a data type in a system view looks like a wrong
choice to me. Or we could actually just use text... Opinions from
others are welcome here of course.

>> I have copied the example file of pg_hba.conf, reloaded it:
>> https://www.postgresql.org/docs/devel/static/auth-pg-hba-conf.html
>> And then the output result gets corrupted by showing up free()'d results:
>> null   | null    | \x7F\x7F\x7F\x7F\x7F
>
> There was a problem in resetting the error string, working with attached
> patch.

Thanks. Now that works.

> Updated patch attached.

This begins to look good. I have found a couple of minor issues.

+  <para>
+   The <structname>pg_hba_rules</structname> view can be read only by
+   superusers.
+  </para>
This is not true anymore.

+     <entry>
+      Line number within client authentication configuration file
+      the current value was set at
+     </entry>
I'd tune that without a past sentence. Saying just pg_hba.conf would
be fine perhaps?

+    <row>
+     <entry><structfield>database</structfield></entry>
+     <entry><structfield>text[]</structfield></entry>
+     <entry>List of database name</entry>
+    </row>
This should be plural, database nameS.

+     <entry>
+      List of keyword address names,
+      name can be all, samehost and samenet
+     </entry>
Phrasing looks weird to me, what about "List of keyword address names,
whose values can be all, samehost or samenet", with <literal> markups.

+postgres=# select line_number, type, database, user_name, auth_method
from pg_hba_rules;
Nit: this could be upper-cased.

+static Datum
+getauthmethod(UserAuth auth_method)
+{
+ ...
+       default:
+           elog(ERROR, "unexpected authentication method in parsed HBA entry");
+           break;
+   }
I think that you should also remove the default clause here to catchup
errors at compilation.

+       switch (hba->conntype)
+       {
+           case ctLocal:
+               values[index] = CStringGetTextDatum("local");
+               break;
+           case ctHost:
+               values[index] = CStringGetTextDatum("host");
+               break;
+           case ctHostSSL:
+               values[index] = CStringGetTextDatum("hostssl");
+               break;
+           case ctHostNoSSL:
+               values[index] = CStringGetTextDatum("hostnossl");
+               break;
+           default:
+               elog(ERROR, "unexpected connection type in parsed HBA entry");
+       }
You could go without the default clause here as well.
-- 
Michael

On Wed, Jan 18, 2017 at 4:11 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> updated patch attached.

Thanks for the new version.

> Added tap tests patch also attached.

This begins to look really nice. I am having fun torturing it :)

Here are I think my last comments:

+   linecxt = tokenize_file(HbaFileName, file, &hba_lines,
&hba_line_nums, &hba_raw_lines);
+   FreeFile(file);
tokenize_file can leave on ERROR, in which case the file descriptor
would leak. You much likely need a
PG_END_ENSURE_ERROR_CLEANUP/PG_ENSURE_ERROR_CLEANUP block here with a
callback to FreeFile() if an error is caught.

+     <entry>
+      ADDRESS specifies the set of hosts the record matches.
+      It can be a host name, or it is made up of an IP address
+      or keywords such as (<literal>all</literal>,
+      <literal>samehost</literal> and <literal>samenet</literal>).
+     </entry>
Why is that upper-case?

+     <entry>
+      If not null, an error message indicating why this
+      rule could not be loaded
+     </entry>
Need a dot here, that's a sentence.

src/test/regress/expected/rules.out needs to be refreshed, regression
tests are failing.
-- 
Michael

On Thu, Jan 19, 2017 at 4:25 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> Added the cleanup mechanism. But the tokenize_file() function call
> present in many places, But in one flow still it is possible to have
> file descriptor leak because of pg_hba_rules view. Because of this
> reason, added the cleanup everywhere.

Oops, sorry. Actually you don't need that. AllocateFile() registers
the fd opened with the sub-transactions it is involved with... So if
there is an ERROR nothing leaks.
-- 
Michael

On Thu, Jan 19, 2017 at 1:26 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Thu, Jan 19, 2017 at 4:25 PM, Haribabu Kommi
> <kommi.haribabu@gmail.com> wrote:
>> Added the cleanup mechanism. But the tokenize_file() function call
>> present in many places, But in one flow still it is possible to have
>> file descriptor leak because of pg_hba_rules view. Because of this
>> reason, added the cleanup everywhere.
>
> Oops, sorry. Actually you don't need that. AllocateFile() registers
> the fd opened with the sub-transactions it is involved with... So if
> there is an ERROR nothing leaks.

I agree. If we need any fix, it should be a separate patch.

The patch is in much better shape than previous versions. Thanks for
working on it.

Here are some more review comments.
'indicates' should be used instead of 'indicating'
+  <para>
+   If the configuration file contains any problems,
<structfield>error</structfield> field
+   indicating the problem of that rule. Following is the sample
output of the view.
+  </para>
The first sentence may be rewritten as
<structfield>error</structfield> field, if not NULL, describes problem in
the rule on the line <structfield>line_number</structfield>.

Instead of showing same values like {all}, trust on multiple lines, you may
show an example with different values on different lines.
+<screen>
+ line_number | type  | database | user_name | auth_method
+-------------+-------+----------+-----------+-------------
+          84 | local | {all}    | {all}     | trust
+          86 | host  | {all}    | {all}     | trust
+          88 | host  | {all}    | {all}     | trust
+(3 rows)
+</screen>

getauthmethod() deparses the authentication tokens parsed in parse_hba_line()
starting with /* Get the authentication method */. There is less chance that
those tokens would be changed later, but we might need adjustments when new
methods are added or method names are changed. Instead, we might want to create
an array of token where nth token indicates auth_method = n. The code block in
parse_hba_line() can be changed to look up this array and assign index of the
token if found to auth_method. Token which are enabled by compiler flags will
be part of the array only when that flag is enabled, otherwise they will be
NULL.
#ifdef ENABLE_GSS       parsedline->auth_method = uaGSS;
#else       unsupauth = "gss";
#endif
If we do that getauthmethod() simply fetches the token by referencing array
with auth_method as index, with some special handling for uaImplicitReject.
This will take away any future maintenance needed. Something similar can be
done to conntype.

This is not going to help in binary without CASSERT i.e. for most users, if
they provide more than 12 options, albeit resulting in an error. Please convert
this into an elog() or another error that hba parser throws.
+    Assert(noptions <= MAX_OPTIONS);
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Thu, Jan 19, 2017 at 9:28 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Thu, Jan 19, 2017 at 1:26 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> On Thu, Jan 19, 2017 at 4:25 PM, Haribabu Kommi
>> <kommi.haribabu@gmail.com> wrote:
>>> Added the cleanup mechanism. But the tokenize_file() function call
>>> present in many places, But in one flow still it is possible to have
>>> file descriptor leak because of pg_hba_rules view. Because of this
>>> reason, added the cleanup everywhere.
>>
>> Oops, sorry. Actually you don't need that. AllocateFile() registers
>> the fd opened with the sub-transactions it is involved with... So if
>> there is an ERROR nothing leaks.
>
> I agree. If we need any fix, it should be a separate patch.

It happens that no fix is needed here. That was some useless fuss. Sorry.
-- 
Michael

On Fri, Jan 20, 2017 at 10:56 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> The Assert case can be hit only, when the user added to new options to
> display
> to the user through view but not updating the macro to the max number of
> options then, it can lead to that assert.
>
> Updated patch attached including reverting of file leak changes.

OK, thanks for the new version. I am marking this version as ready for
committer.
-- 
Michael

On Fri, Jan 20, 2017 at 12:46 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Fri, Jan 20, 2017 at 10:56 AM, Haribabu Kommi
> <kommi.haribabu@gmail.com> wrote:
>> The Assert case can be hit only, when the user added to new options to
>> display
>> to the user through view but not updating the macro to the max number of
>> options then, it can lead to that assert.
>>
>> Updated patch attached including reverting of file leak changes.
>
> OK, thanks for the new version. I am marking this version as ready for
> committer.

I do intend to make a pass ASAP.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> [ pg_hba_rules_10.patch ]

I took a quick look over this.

* I'm not exactly convinced that the way you approached the error message
reporting, ie duplicating the logged message, is good.  In particular
this results in localizing the strings reported in pg_hba_rules.error,
which is exactly opposite to the decision we reached for the
pg_file_settings view.  What's the reasoning for deciding that this
view should contain localized strings?  (More generally, we found in
the pg_file_settings view that we didn't always want to use exactly
the same string that was logged, anyway.)

* Also, there seems to be a lot of ereports remaining unconverted,
eg the "authentication file token too long" error.  One of the things
we wanted pg_file_settings to be able to do was finger pretty much any
mistake in the config file, including syntax errors.  It seems like
it'd be a shame if pg_hba_rules is unable to help with that.  You
should be able to fill in line number and error even if the line is
too mangled to be able to populate the other fields sanely.

* While we're on the comparison to pg_file_settings ... pg_hba_rules
is not the view name I'd guess if I guessed one based on that precedent.
I don't have a better suggestion offhand, but this name seems weirdly
inconsistent.

* I think "memcxt" as a field name is pretty unhelpful if you suppose
it just means "memory context", and outright misleading if you guess
it's, say, the context the tuplestore is in.  Perhaps call it "tmpcxt"
and add a comment like "Short-lived context, reset after each line".
The other fields of FillHbaLineCxt could do with comments too.

* ... although really, you've gone way overboard with temp contexts
here.  I don't think it's necessary to have a per-line context at all;
you could just do all the work in the single temp context that fill_hba
calls hbacxt, and drop it all at end of function, because no matter what
you'll be eating O(file size) space, and we're just quibbling over the
size of the multiplier.  Also, if you're concerned with reclaiming space
before end of query, aren't you leaking the tokenize_file output data?

* getauthmethod() might be better replaced with an array.  And doesn't it
produce uninitialized-variable warnings for you?

* It seems a little weird that fill_hba_auth_opt isn't inserting the "="
between name and value.  And should it be using psprintf?  It's the
only use of StringInfo in this file, so it looks a bit out of place.
Actually, I wonder if you wouldn't be better off replacing it with a
coding style like
options[noptions++] =    CStringGetTextDatum(psprintf("ldapport=%d", hba->ldapport));

which seems more readable and more flexible.

* "MAX_OPTIONS" is, uh, mighty generic.  Maybe "MAX_HBA_OPTIONS"?
And the comment for it doesn't actually tell you what it is.

* NUM_PG_HBA_LOOKUP_ATTS seems like it ought to match the name of the
view, ie NUM_PG_HBA_RULES_ATTS if that doesn't get renamed.

* Usually we just write "if (listvar)" or "if (listvar != NIL)" rather
than "if (list_length(listvar) != 0)".  list_length() overspecifies what
you need to test.  This isn't as critical as it was back in the day when
list_length() cost O(N), but still the former is much more common project
style.

* Why is AllocateFile() failure only an ereport(LOG) in fill_hba()?
From the user's viewpoint he'll get an empty view with no visible
reason.  Probably ereport(ERROR) is more sensible.  You could imagine
trying to show the error in the view, but that seems like more work
than the case warrants.

* Seems like the FillHbaLineCxt variable could just be a local struct
in hba_rules(), and dispense with one palloc/pfree cycle.

* I'm not really on board with patches modifying pgindent/typedefs.list
retail.  To my mind that file represents the typedefs used the last
time we pgindent'd the whole tree, and if you want an up-to-date list
you should ask the buildfarm.  Otherwise there's just too much confusion
stemming from the fact that not everybody updates it when patching.

My own workflow for reindenting patches goes more like
curl https://buildfarm.postgresql.org/cgi-bin/typedefs.pl -o my-typedefs.list
... manually edit my-typedefs.list to add any new typedefs from patch ...
pgindent --typedefs=my-typedefs.list target-files
        regards, tom lane

On Mon, Jan 23, 2017 at 5:13 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> * I'm not exactly convinced that the way you approached the error message
>> reporting, ie duplicating the logged message, is good.  In particular
>> this results in localizing the strings reported in pg_hba_rules.error,
>> which is exactly opposite to the decision we reached for the
>> pg_file_settings view.  What's the reasoning for deciding that this
>> view should contain localized strings?  (More generally, we found in
>> the pg_file_settings view that we didn't always want to use exactly
>> the same string that was logged, anyway.)
>
> Actually there is no particular reason to display the localized strings,
> Just thought that it may be useful to the user if it get displayed in their
> own language. And also doing this way will reduce the error message
> duplicate in the code that is used for display in the view and writing it
> into the log file.

Perhaps consistency would not hurt and something like
record_config_file_error() could be done to save the error parsing
error. What's actually the problem with localized strings exposed in a
system view? Encoding conflicts?

>> * Also, there seems to be a lot of ereports remaining unconverted,
>> eg the "authentication file token too long" error.  One of the things
>> we wanted pg_file_settings to be able to do was finger pretty much any
>> mistake in the config file, including syntax errors.  It seems like
>> it'd be a shame if pg_hba_rules is unable to help with that.  You
>> should be able to fill in line number and error even if the line is
>> too mangled to be able to populate the other fields sanely.
>
> The two errors that are missed are, "could not open secondary authentication
> file"
> and "authentication file token too long" errors. For these two cases, the
> server
> is not throwing any error, it just logs the message and continues. Is it
> fine to add
> these these two cases as errors in the view?

Missed those ones during the initial review... It would be a good idea
to include them to track problems.
-- 
Michael

On Mon, Jan 23, 2017 at 1:43 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>> > [ pg_hba_rules_10.patch ]
>>
>> I took a quick look over this.
>
>
> Thanks for the review.
>
>>
>> * I'm not exactly convinced that the way you approached the error message
>> reporting, ie duplicating the logged message, is good.  In particular
>> this results in localizing the strings reported in pg_hba_rules.error,
>> which is exactly opposite to the decision we reached for the
>> pg_file_settings view.  What's the reasoning for deciding that this
>> view should contain localized strings?  (More generally, we found in
>> the pg_file_settings view that we didn't always want to use exactly
>> the same string that was logged, anyway.)
>
>
> Actually there is no particular reason to display the localized strings,
> Just thought that it may be useful to the user if it get displayed in their
> own language. And also doing this way will reduce the error message
> duplicate in the code that is used for display in the view and writing it
> into the log file.
>

Would it be better, if we could parse each HBA line within
PG_TRY()/PG_CATCH() and read errmsg from errordata stack in
PG_CATCH()? We do that only when errcode is ERRCODE_CONFIG_FILE_ERROR,
PG_THROWing otherwise. That's probably a bad idea but wanted to put it
out as it came to me. It would eliminate a lot of changes in this
patch.


>> * getauthmethod() might be better replaced with an array.  And doesn't it
>> produce uninitialized-variable warnings for you?
>
>
> No, i am not getting any warnings.
> Changed to a static array.

Thanks. Probably we should update parse_hba_line() to keep it in sync
with the array. But that may be a separate add-on patch.

Rest of the patch looks good to me.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Tue, Jan 24, 2017 at 11:19 PM, Ashutosh Bapat
<ashutosh.bapat@enterprisedb.com> wrote:
> On Mon, Jan 23, 2017 at 1:43 PM, Haribabu Kommi
> <kommi.haribabu@gmail.com> wrote:
>>
>>
>> On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>
>>> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>>> > [ pg_hba_rules_10.patch ]
>>>
>>> I took a quick look over this.
>>
>>
>> Thanks for the review.
>>
>>>
>>> * I'm not exactly convinced that the way you approached the error message
>>> reporting, ie duplicating the logged message, is good.  In particular
>>> this results in localizing the strings reported in pg_hba_rules.error,
>>> which is exactly opposite to the decision we reached for the
>>> pg_file_settings view.  What's the reasoning for deciding that this
>>> view should contain localized strings?  (More generally, we found in
>>> the pg_file_settings view that we didn't always want to use exactly
>>> the same string that was logged, anyway.)
>>
>>
>> Actually there is no particular reason to display the localized strings,
>> Just thought that it may be useful to the user if it get displayed in their
>> own language. And also doing this way will reduce the error message
>> duplicate in the code that is used for display in the view and writing it
>> into the log file.
>>
>
> Would it be better, if we could parse each HBA line within
> PG_TRY()/PG_CATCH() and read errmsg from errordata stack in
> PG_CATCH()? We do that only when errcode is ERRCODE_CONFIG_FILE_ERROR,
> PG_THROWing otherwise. That's probably a bad idea but wanted to put it
> out as it came to me. It would eliminate a lot of changes in this
> patch.

It still needs to save the error message string somewhere. So I am not
sure that it would save much patch size.
-- 
Michael

On Wed, Jan 25, 2017 at 6:34 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Tue, Jan 24, 2017 at 11:19 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>> On Mon, Jan 23, 2017 at 1:43 PM, Haribabu Kommi
>> <kommi.haribabu@gmail.com> wrote:
>>>
>>>
>>> On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>
>>>> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>>>> > [ pg_hba_rules_10.patch ]
>>>>
>>>> I took a quick look over this.
>>>
>>>
>>> Thanks for the review.
>>>
>>>>
>>>> * I'm not exactly convinced that the way you approached the error message
>>>> reporting, ie duplicating the logged message, is good.  In particular
>>>> this results in localizing the strings reported in pg_hba_rules.error,
>>>> which is exactly opposite to the decision we reached for the
>>>> pg_file_settings view.  What's the reasoning for deciding that this
>>>> view should contain localized strings?  (More generally, we found in
>>>> the pg_file_settings view that we didn't always want to use exactly
>>>> the same string that was logged, anyway.)
>>>
>>>
>>> Actually there is no particular reason to display the localized strings,
>>> Just thought that it may be useful to the user if it get displayed in their
>>> own language. And also doing this way will reduce the error message
>>> duplicate in the code that is used for display in the view and writing it
>>> into the log file.
>>>
>>
>> Would it be better, if we could parse each HBA line within
>> PG_TRY()/PG_CATCH() and read errmsg from errordata stack in
>> PG_CATCH()? We do that only when errcode is ERRCODE_CONFIG_FILE_ERROR,
>> PG_THROWing otherwise. That's probably a bad idea but wanted to put it
>> out as it came to me. It would eliminate a lot of changes in this
>> patch.
>
> It still needs to save the error message string somewhere. So I am not
> sure that it would save much patch size.

My understanding is that ereport (and some other calls included in
that statement) call saves it on errordata stack before jumping to the
handler.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

On Wed, Jan 25, 2017 at 9:58 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Wed, Jan 25, 2017 at 2:50 PM, Ashutosh Bapat
> <ashutosh.bapat@enterprisedb.com> wrote:
>>
>> On Wed, Jan 25, 2017 at 6:34 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>> > On Tue, Jan 24, 2017 at 11:19 PM, Ashutosh Bapat
>> > <ashutosh.bapat@enterprisedb.com> wrote:
>> >> On Mon, Jan 23, 2017 at 1:43 PM, Haribabu Kommi
>> >> <kommi.haribabu@gmail.com> wrote:
>> >>>
>> >>>
>> >>> On Sat, Jan 21, 2017 at 8:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> >>>>
>> >>>> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>> >>>> > [ pg_hba_rules_10.patch ]
>> >>>>
>> >>>> I took a quick look over this.
>> >>>
>> >>>
>> >>> Thanks for the review.
>> >>>
>> >>>>
>> >>>> * I'm not exactly convinced that the way you approached the error
>> >>>> message
>> >>>> reporting, ie duplicating the logged message, is good.  In particular
>> >>>> this results in localizing the strings reported in
>> >>>> pg_hba_rules.error,
>> >>>> which is exactly opposite to the decision we reached for the
>> >>>> pg_file_settings view.  What's the reasoning for deciding that this
>> >>>> view should contain localized strings?  (More generally, we found in
>> >>>> the pg_file_settings view that we didn't always want to use exactly
>> >>>> the same string that was logged, anyway.)
>> >>>
>> >>>
>> >>> Actually there is no particular reason to display the localized
>> >>> strings,
>> >>> Just thought that it may be useful to the user if it get displayed in
>> >>> their
>> >>> own language. And also doing this way will reduce the error message
>> >>> duplicate in the code that is used for display in the view and writing
>> >>> it
>> >>> into the log file.
>> >>>
>> >>
>> >> Would it be better, if we could parse each HBA line within
>> >> PG_TRY()/PG_CATCH() and read errmsg from errordata stack in
>> >> PG_CATCH()? We do that only when errcode is ERRCODE_CONFIG_FILE_ERROR,
>> >> PG_THROWing otherwise. That's probably a bad idea but wanted to put it
>> >> out as it came to me. It would eliminate a lot of changes in this
>> >> patch.
>> >
>> > It still needs to save the error message string somewhere. So I am not
>> > sure that it would save much patch size.
>>
>> My understanding is that ereport (and some other calls included in
>> that statement) call saves it on errordata stack before jumping to the
>> handler.
>
>
> All the ereport messages of level are LOG, because of this reason, because
> of this reason even if we use the TRY/CATCH, it doesn't work.  As the
> messages gets printed to the logfile and continue to process the next
> statement.

Right. Sorry for missing to mention about this change in the patch.
Originally the messages are at level ERROR so TRY/CATCH will be able
to catch it. We will need to somehow then turn ERROR to LOG and
re-throw it. I haven't tried it myself though.

-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> On Wed, Jan 25, 2017 at 9:58 AM, Haribabu Kommi
> <kommi.haribabu@gmail.com> wrote:
>> All the ereport messages of level are LOG, because of this reason, because
>> of this reason even if we use the TRY/CATCH, it doesn't work.  As the
>> messages gets printed to the logfile and continue to process the next
>> statement.

> Right. Sorry for missing to mention about this change in the patch.
> Originally the messages are at level ERROR so TRY/CATCH will be able
> to catch it. We will need to somehow then turn ERROR to LOG and
> re-throw it. I haven't tried it myself though.

I do not think throwing/catching errors is a good idea here.  It will mean
that the view can't report more than one mistake per run, and it will
create a significant difference in the parsing code's control flow between
"normal" and "read for view" modes, which is a recipe for bugs.  Also,
it's different from the way things are done for the pg_file_settings view.
For the sake of future developers, I think we should make this work as
much like that view as we can.

The way I'd be inclined to make the individual reporting changes is like
            if (!EnableSSL)
+            {
-               ereport(LOG,
+               ereport(elevel,                        (errcode(ERRCODE_CONFIG_FILE_ERROR),
errmsg("hostsslrecord cannot match because SSL is disabled"),                         errhint("Set ssl = on in
postgresql.conf."),                        errcontext("line %d of configuration file \"%s\"",
        line_num, HbaFileName))); 
+                *err_msg = pstrdup("hostssl record cannot match because SSL is disabled");
+            }

which is considerably less invasive and hence easier to review, and
supports reporting different text in the view than appears in the log,
should we need that.  It seems likely also that we could drop the pstrdup
in the case of constant strings (we'd still need psprintf if we want to
insert values into the view messages), which would make this way cheaper
than what's in the patch now.
        regards, tom lane

On Thu, Jan 26, 2017 at 2:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> The way I'd be inclined to make the individual reporting changes is like
>
>              if (!EnableSSL)
> +            {
> -               ereport(LOG,
> +               ereport(elevel,
>                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
>                 errmsg("hostssl record cannot match because SSL is disabled"),
>                           errhint("Set ssl = on in postgresql.conf."),
>                           errcontext("line %d of configuration file \"%s\"",
>                                      line_num, HbaFileName)));
> +                *err_msg = pstrdup("hostssl record cannot match because SSL is disabled");
> +            }
>
> which is considerably less invasive and hence easier to review, and
> supports reporting different text in the view than appears in the log,
> should we need that.  It seems likely also that we could drop the pstrdup
> in the case of constant strings (we'd still need psprintf if we want to
> insert values into the view messages), which would make this way cheaper
> than what's in the patch now.

I don't really understand the argument about readability of the patch
as what Haribabu has proposed is simply to avoid a duplicate of the
strings and the diffs of the patch are really clear. For the sake of
not translating the strings sent back to the system view though I can
buy it.
-- 
Michael

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> This patch currently doesn't have the code for reporting the two log
> messages that can occur in tokenize_file function. To support the same,
> I am thinking of changing line_nums list to line_info list that can
> contain both line number and the error message that occurred during the
> tokenize. This list data is used to identify whether that line is having
> any error or not before parsing that hba line, and directly report that
> line as error in the view.

Yeah, perhaps.  tokenize_file() has pushed the return-parallel-lists
notion to the limit of sanity already.  It would make more sense to
change it to return a single list containing one struct per line,
which would include the token list, raw line text, and line number.

It might make sense to proceed by writing a separate patch that just
refactors the existing code to have an API like that, and then revise
this patch to add an error message field to the per-line struct.  Or
maybe that's just extra work, not sure.
        regards, tom lane

On Thu, Jan 26, 2017 at 11:36 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>> This patch currently doesn't have the code for reporting the two log
>> messages that can occur in tokenize_file function. To support the same,
>> I am thinking of changing line_nums list to line_info list that can
>> contain both line number and the error message that occurred during the
>> tokenize. This list data is used to identify whether that line is having
>> any error or not before parsing that hba line, and directly report that
>> line as error in the view.
>
> Yeah, perhaps.  tokenize_file() has pushed the return-parallel-lists
> notion to the limit of sanity already.  It would make more sense to
> change it to return a single list containing one struct per line,
> which would include the token list, raw line text, and line number.
>
> It might make sense to proceed by writing a separate patch that just
> refactors the existing code to have an API like that, and then revise
> this patch to add an error message field to the per-line struct.  Or
> maybe that's just extra work, not sure.

Beginning with a cleaner state the feature implementation would likely
facilitate the restructuring work of pg_hba_rules and its overall
size, so doing the refactoring work first would make the most sense.
-- 
Michael

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> On Fri, Jan 27, 2017 at 1:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It might make sense to proceed by writing a separate patch that just
>> refactors the existing code to have an API like that, and then revise
>> this patch to add an error message field to the per-line struct.  Or
>> maybe that's just extra work, not sure.

> Here I attached tokenize_file refactor patch to return single linked list
> that contains a structure and rebased pg_hba_rules patch on top it
> by adding an error message to that structure to hold the errors occurred
> during tokenization..

I pushed the first patch with some revisions.  You had the TokenizedLine
struct containing something that was still a three-level-nesting list,
whereas it only needs to be two levels, and you hadn't updated any of
the comments that the patch falsified.  Also, I figured we might as well
pass the TokenizedLine struct as-is to parse_hba_line and
parse_ident_line, because that was going to be the next step anyway
so they could pass back error messages.

Off to look at the second patch ...
        regards, tom lane

On Fri, Jan 20, 2017 at 4:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> * I'm not really on board with patches modifying pgindent/typedefs.list
> retail.  To my mind that file represents the typedefs used the last
> time we pgindent'd the whole tree, and if you want an up-to-date list
> you should ask the buildfarm.  Otherwise there's just too much confusion
> stemming from the fact that not everybody updates it when patching.
>
> My own workflow for reindenting patches goes more like
> curl https://buildfarm.postgresql.org/cgi-bin/typedefs.pl -o my-typedefs.list
> ... manually edit my-typedefs.list to add any new typedefs from patch ...
> pgindent --typedefs=my-typedefs.list target-files

Andres and I -- among others -- have been patching typedefs.list
retail for a while now.  I think it makes life a lot easier.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> [ pg_hba_rules_13.patch ]

I spent awhile hacking on this, and made a lot of things better, but
I'm still very unhappy about the state of the comments.  You changed
the APIs of a bunch of functions, often into fairly subtle things,
and you did not touch even one of their API-specification comments.
As an example, next_token() now needs something like

"On error, log a message at ereport level elevel and set *err_msg to
an error string.  Note that the return value might be either true or
false after an error; *err_msg must be checked to determine that.
Hence, *err_msg had better be NULL on entry, or you won't be able
to tell."

Having to write such a thing might even convince you that you should
try a little harder to make the behavior less confusing.  Just adding
arguments, and not changing the result-value specification, is not
necessarily the best way to do this.

I haven't looked at the docs yet.

I'm still not very happy about the choice of view name ...

            regards, tom lane

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 086fafc..3f4724c 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***************
*** 7804,7809 ****
--- 7804,7814 ----
       </row>

       <row>
+       <entry><link linkend="view-pg-hba-rules"><structname>pg_hba_rules</structname></link></entry>
+       <entry>summary of client authentication configuration file contents</entry>
+      </row>
+
+      <row>
        <entry><link linkend="view-pg-group"><structname>pg_group</structname></link></entry>
        <entry>groups of database users</entry>
       </row>
***************
*** 8352,8357 ****
--- 8357,8481 ----

  </sect1>

+  <sect1 id="view-pg-hba-rules">
+   <title><structname>pg_hba_rules</structname></title>
+
+   <indexterm zone="view-pg-hba-rules">
+    <primary>pg_hba_rules</primary>
+   </indexterm>
+
+   <para>
+    The view <structname>pg_hba_rules</structname> provides a summary of
+    the contents of the client authentication configuration file.  A row
+    appears in this view for each entry appearing in the file, with annotations
+    indicating whether the rule could be applied successfully.
+   </para>
+
+   <table>
+    <title><structname>pg_hba_rules</> Columns</title>
+
+   <tgroup cols="3">
+    <thead>
+     <row>
+      <entry>Name</entry>
+      <entry>Type</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry><structfield>line_number</structfield></entry>
+      <entry><structfield>integer</structfield></entry>
+      <entry>
+       Line number of the client authentication rule in
+       pg_hba.conf file
+      </entry>
+     </row>
+     <row>
+      <entry><structfield>type</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>Type of connection</entry>
+     </row>
+     <row>
+      <entry><structfield>database</structfield></entry>
+      <entry><structfield>text[]</structfield></entry>
+      <entry>List of database names</entry>
+     </row>
+     <row>
+      <entry><structfield>user_name</structfield></entry>
+      <entry><structfield>text[]</structfield></entry>
+      <entry>List of user names</entry>
+     </row>
+     <row>
+      <entry><structfield>address</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>
+       Address specifies the set of hosts the record matches.
+       It can be a host name, or it is made up of an IP address
+       or keywords such as (<literal>all</literal>,
+       <literal>samehost</literal> and <literal>samenet</literal>).
+      </entry>
+     </row>
+     <row>
+      <entry><structfield>netmask</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>Address mask if exist</entry>
+     </row>
+     <row>
+      <entry><structfield>auth_method</structfield></entry>
+      <entry><type>text</type></entry>
+      <entry>Authentication method</entry>
+     </row>
+     <row>
+      <entry><structfield>options</structfield></entry>
+      <entry><type>text[]</type></entry>
+      <entry>Configuration options set for authentication method</entry>
+     </row>
+     <row>
+      <entry><structfield>error</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>
+       If not null, an error message indicates why this
+       rule could not be loaded.
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+   </table>
+
+   <para>
+    <structfield>error</structfield> field, if not NULL, describes problem
+    in the rule on the line <structfield>line_number</structfield>.
+    Following is the sample output of the view.
+   </para>
+
+ <programlisting>
+ SELECT line_number, type, database, user_name, auth_method FROM pg_hba_rules;
+ </programlisting>
+
+ <screen>
+  line_number | type  |  database  | user_name  |   address    | auth_method
+ -------------+-------+------------+------------+--------------+-------------
+           84 | local | {all}      | {all}      |              | trust
+           86 | host  | {sameuser} | {postgres} | all          | trust
+           88 | host  | {postgres} | {postgres} | ::1          | trust
+          111 | host  | {all}      | {all}      | 127.0.0.1    | trust
+          121 | host  | {all}      | {all}      | localhost    | trust
+          128 | host  | {postgres} | {all}      | samenet      | ident
+          134 | host  | {postgres} | {all}      | samehost     | md5
+          140 | host  | {db1,db2}  | {all}      | .example.com | md5
+          149 | host  | {test}     | {test}     | 192.168.54.1 | reject
+          159 | host  | {all}      | {+support} | 192.168.0.0  | ident
+          169 | local | {sameuser} | {all}      |              | md5
+ (11 rows)
+ </screen>
+
+   <para>
+    See <xref linkend="client-authentication"> for more information about the various
+    ways to change client authentication configuration.
+   </para>
+  </sect1>
+
   <sect1 id="view-pg-group">
    <title><structname>pg_group</structname></title>

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index dda5891..f20486c 100644
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
***************
*** 54,59 ****
--- 54,66 ----
    database user names and OS user names.
   </para>

+  <para>
+   The system view
+   <link linkend="view-pg-hba-rules"><structname>pg_hba_rules</structname></link>
+   can be helpful for pre-testing changes to the client authentication configuration file, or for
+   diagnosing problems if loading of file did not have the desired effects.
+  </para>
+
   <sect1 id="auth-pg-hba-conf">
    <title>The <filename>pg_hba.conf</filename> File</title>

diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 4dfedf8..d920a72 100644
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*************** CREATE VIEW pg_file_settings AS
*** 459,464 ****
--- 459,470 ----
  REVOKE ALL on pg_file_settings FROM PUBLIC;
  REVOKE EXECUTE ON FUNCTION pg_show_all_file_settings() FROM PUBLIC;

+ CREATE VIEW pg_hba_rules AS
+    SELECT * FROM pg_hba_rules() AS A;
+
+ REVOKE ALL on pg_hba_rules FROM PUBLIC;
+ REVOKE EXECUTE ON FUNCTION pg_hba_rules() FROM PUBLIC;
+
  CREATE VIEW pg_timezone_abbrevs AS
      SELECT * FROM pg_timezone_abbrevs();

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index bbe0a88..289bd9d 100644
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***************
*** 25,39 ****
--- 25,46 ----
  #include <arpa/inet.h>
  #include <unistd.h>

+ #include "access/htup_details.h"
+ #include "catalog/objectaddress.h"
  #include "catalog/pg_collation.h"
+ #include "catalog/pg_type.h"
  #include "common/ip.h"
+ #include "funcapi.h"
  #include "libpq/ifaddr.h"
  #include "libpq/libpq.h"
+ #include "miscadmin.h"
  #include "postmaster/postmaster.h"
  #include "regex/regex.h"
  #include "replication/walsender.h"
  #include "storage/fd.h"
+ #include "storage/ipc.h"
  #include "utils/acl.h"
+ #include "utils/builtins.h"
  #include "utils/guc.h"
  #include "utils/lsyscache.h"
  #include "utils/memutils.h"
*************** typedef struct HbaToken
*** 80,91 ****
--- 87,101 ----
   * Each item in the "fields" list is a sub-list of HbaTokens.
   * We don't emit a TokenizedLine for empty or all-comment lines,
   * so "fields" is never NIL (nor are any of its sub-lists).
+  * Exception: if an error occurs during tokenization, we might
+  * have fields == NIL, in which case err_msg != NULL.
   */
  typedef struct TokenizedLine
  {
      List       *fields;            /* List of lists of HbaTokens */
      int            line_num;        /* Line number */
      char       *raw_line;        /* Raw line text */
+     char       *err_msg;        /* Error message if any */
  } TokenizedLine;

  /*
*************** static MemoryContext parsed_hba_context
*** 106,118 ****
  static List *parsed_ident_lines = NIL;
  static MemoryContext parsed_ident_context = NULL;


  static MemoryContext tokenize_file(const char *filename, FILE *file,
!               List **tok_lines);
  static List *tokenize_inc_file(List *tokens, const char *outer_filename,
!                   const char *inc_filename);
  static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
!                    int line_num);

  /*
   * isblank() exists in the ISO C99 spec, but it's not very portable yet,
--- 116,157 ----
  static List *parsed_ident_lines = NIL;
  static MemoryContext parsed_ident_context = NULL;

+ /*
+  * The following character array represents the names of the authentication
+  * methods that are supported by PostgreSQL.
+  *
+  * Note: keep this in sync with the UserAuth enum in hba.h.
+  */
+ static const char *const UserAuthName[] =
+ {
+     "reject",
+     "implicit reject",            /* Not a user-visible option */
+     "trust",
+     "ident",
+     "password",
+     "md5",
+     "gss",
+     "sspi",
+     "pam",
+     "bsd",
+     "ldap",
+     "cert",
+     "radius",
+     "peer"
+ };
+

  static MemoryContext tokenize_file(const char *filename, FILE *file,
!               List **tok_lines, int elevel);
  static List *tokenize_inc_file(List *tokens, const char *outer_filename,
!                   const char *inc_filename, int elevel, char **err_msg);
  static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
!                    int elevel, char **err_msg);
! static ArrayType *gethba_options(HbaLine *hba);
! static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
!               int lineno, HbaLine *hba, const char *err_msg);
! static void fill_hba(Tuplestorestate *tuple_store, TupleDesc tupdesc);
!

  /*
   * isblank() exists in the ISO C99 spec, but it's not very portable yet,
*************** pg_isblank(const char c)
*** 151,157 ****
   */
  static bool
  next_token(char **lineptr, char *buf, int bufsz, bool *initial_quote,
!            bool *terminating_comma)
  {
      int            c;
      char       *start_buf = buf;
--- 190,196 ----
   */
  static bool
  next_token(char **lineptr, char *buf, int bufsz, bool *initial_quote,
!            bool *terminating_comma, int elevel, char **err_msg)
  {
      int            c;
      char       *start_buf = buf;
*************** next_token(char **lineptr, char *buf, in
*** 197,206 ****
          if (buf >= end_buf)
          {
              *buf = '\0';
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                 errmsg("authentication file token too long, skipping: \"%s\"",
                        start_buf)));
              /* Discard remainder of line */
              while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
                  ;
--- 236,246 ----
          if (buf >= end_buf)
          {
              *buf = '\0';
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                 errmsg("authentication file token too long, skipping: \"%s\"",
                        start_buf)));
+             *err_msg = "authentication file token too long";
              /* Discard remainder of line */
              while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
                  ;
*************** copy_hba_token(HbaToken *in)
*** 279,285 ****
   * or NIL if we reached EOL.
   */
  static List *
! next_field_expand(const char *filename, char **lineptr)
  {
      char        buf[MAX_TOKEN];
      bool        trailing_comma;
--- 319,325 ----
   * or NIL if we reached EOL.
   */
  static List *
! next_field_expand(const char *filename, char **lineptr, int elevel, char **err_msg)
  {
      char        buf[MAX_TOKEN];
      bool        trailing_comma;
*************** next_field_expand(const char *filename,
*** 288,302 ****

      do
      {
!         if (!next_token(lineptr, buf, sizeof(buf), &initial_quote, &trailing_comma))
              break;

          /* Is this referencing a file? */
          if (!initial_quote && buf[0] == '@' && buf[1] != '\0')
!             tokens = tokenize_inc_file(tokens, filename, buf + 1);
          else
              tokens = lappend(tokens, make_hba_token(buf, initial_quote));
!     } while (trailing_comma);

      return tokens;
  }
--- 328,342 ----

      do
      {
!         if (!next_token(lineptr, buf, sizeof(buf), &initial_quote, &trailing_comma, elevel, err_msg) || (*err_msg !=
NULL))
              break;

          /* Is this referencing a file? */
          if (!initial_quote && buf[0] == '@' && buf[1] != '\0')
!             tokens = tokenize_inc_file(tokens, filename, buf + 1, elevel, err_msg);
          else
              tokens = lappend(tokens, make_hba_token(buf, initial_quote));
!     } while (trailing_comma && (*err_msg == NULL));

      return tokens;
  }
*************** next_field_expand(const char *filename,
*** 313,319 ****
  static List *
  tokenize_inc_file(List *tokens,
                    const char *outer_filename,
!                   const char *inc_filename)
  {
      char       *inc_fullname;
      FILE       *inc_file;
--- 353,361 ----
  static List *
  tokenize_inc_file(List *tokens,
                    const char *outer_filename,
!                   const char *inc_filename,
!                   int elevel,
!                   char **err_msg)
  {
      char       *inc_fullname;
      FILE       *inc_file;
*************** tokenize_inc_file(List *tokens,
*** 340,355 ****
      inc_file = AllocateFile(inc_fullname, "r");
      if (inc_file == NULL)
      {
!         ereport(LOG,
                  (errcode_for_file_access(),
                   errmsg("could not open secondary authentication file \"@%s\" as \"%s\": %m",
                          inc_filename, inc_fullname)));
          pfree(inc_fullname);
          return tokens;
      }

      /* There is possible recursion here if the file contains @ */
!     linecxt = tokenize_file(inc_fullname, inc_file, &inc_lines);

      FreeFile(inc_file);
      pfree(inc_fullname);
--- 382,401 ----
      inc_file = AllocateFile(inc_fullname, "r");
      if (inc_file == NULL)
      {
!         int            save_errno = errno;
!
!         ereport(elevel,
                  (errcode_for_file_access(),
                   errmsg("could not open secondary authentication file \"@%s\" as \"%s\": %m",
                          inc_filename, inc_fullname)));
+         *err_msg = psprintf("could not open secondary authentication file \"@%s\" as \"%s\": %s",
+                             inc_filename, inc_fullname, strerror(save_errno));
          pfree(inc_fullname);
          return tokens;
      }

      /* There is possible recursion here if the file contains @ */
!     linecxt = tokenize_file(inc_fullname, inc_file, &inc_lines, elevel);

      FreeFile(inc_file);
      pfree(inc_fullname);
*************** tokenize_inc_file(List *tokens,
*** 389,395 ****
   * this function (it's a child of caller's context).
   */
  static MemoryContext
! tokenize_file(const char *filename, FILE *file, List **tok_lines)
  {
      int            line_number = 1;
      MemoryContext linecxt;
--- 435,441 ----
   * this function (it's a child of caller's context).
   */
  static MemoryContext
! tokenize_file(const char *filename, FILE *file, List **tok_lines, int elevel)
  {
      int            line_number = 1;
      MemoryContext linecxt;
*************** tokenize_file(const char *filename, FILE
*** 407,422 ****
          char        rawline[MAX_LINE];
          char       *lineptr;
          List       *current_line = NIL;

          if (!fgets(rawline, sizeof(rawline), file))
              break;
          if (strlen(rawline) == MAX_LINE - 1)
              /* Line too long! */
!             ereport(ERROR,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication file line too long"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_number, filename)));

          /* Strip trailing linebreak from rawline */
          lineptr = rawline + strlen(rawline) - 1;
--- 453,472 ----
          char        rawline[MAX_LINE];
          char       *lineptr;
          List       *current_line = NIL;
+         char       *err_msg = NULL;

          if (!fgets(rawline, sizeof(rawline), file))
              break;
          if (strlen(rawline) == MAX_LINE - 1)
+         {
              /* Line too long! */
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication file line too long"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_number, filename)));
+             err_msg = "authentication file line too long";
+         }

          /* Strip trailing linebreak from rawline */
          lineptr = rawline + strlen(rawline) - 1;
*************** tokenize_file(const char *filename, FILE
*** 425,442 ****

          /* Parse fields */
          lineptr = rawline;
!         while (*lineptr)
          {
              List       *current_field;

!             current_field = next_field_expand(filename, &lineptr);
              /* add field to line, unless we are at EOL or comment start */
              if (current_field != NIL)
                  current_line = lappend(current_line, current_field);
          }

          /* Reached EOL; emit line to TokenizedLine list unless it's boring */
!         if (current_line != NIL)
          {
              TokenizedLine *tok_line;

--- 475,493 ----

          /* Parse fields */
          lineptr = rawline;
!         while (*lineptr && err_msg == NULL)
          {
              List       *current_field;

!             current_field = next_field_expand(filename, &lineptr,
!                                               elevel, &err_msg);
              /* add field to line, unless we are at EOL or comment start */
              if (current_field != NIL)
                  current_line = lappend(current_line, current_field);
          }

          /* Reached EOL; emit line to TokenizedLine list unless it's boring */
!         if (current_line != NIL || err_msg != NULL)
          {
              TokenizedLine *tok_line;

*************** tokenize_file(const char *filename, FILE
*** 444,449 ****
--- 495,501 ----
              tok_line->fields = current_line;
              tok_line->line_num = line_number;
              tok_line->raw_line = pstrdup(rawline);
+             tok_line->err_msg = err_msg;
              *tok_lines = lappend(*tok_lines, tok_line);
          }

*************** check_same_host_or_net(SockAddr *raddr,
*** 746,751 ****
--- 798,807 ----

  /*
   * Macros used to check and report on invalid configuration options.
+  * On error: log a message at level elevel, set *err_msg, and exit the function.
+  * These macros are not as general-purpose as they look, because they know
+  * what the calling function's error-exit value is.
+  *
   * INVALID_AUTH_OPTION = reports when an option is specified for a method where it's
   *                         not supported.
   * REQUIRE_AUTH_OPTION = same as INVALID_AUTH_OPTION, except it also checks if the
*************** check_same_host_or_net(SockAddr *raddr,
*** 754,797 ****
   * MANDATORY_AUTH_ARG  = check if a required option is set for an authentication method,
   *                         reporting error if it's not.
   */
! #define INVALID_AUTH_OPTION(optname, validmethods) do {\
!     ereport(LOG, \
              (errcode(ERRCODE_CONFIG_FILE_ERROR), \
               /* translator: the second %s is a list of auth methods */ \
               errmsg("authentication option \"%s\" is only valid for authentication methods %s", \
                      optname, _(validmethods)), \
               errcontext("line %d of configuration file \"%s\"", \
                      line_num, HbaFileName))); \
      return false; \
! } while (0);

! #define REQUIRE_AUTH_OPTION(methodval, optname, validmethods) do {\
      if (hbaline->auth_method != methodval) \
          INVALID_AUTH_OPTION(optname, validmethods); \
! } while (0);

! #define MANDATORY_AUTH_ARG(argvar, argname, authname) do {\
!     if (argvar == NULL) {\
!         ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
                   errmsg("authentication method \"%s\" requires argument \"%s\" to be set", \
                          authname, argname), \
                   errcontext("line %d of configuration file \"%s\"", \
                          line_num, HbaFileName))); \
          return NULL; \
      } \
! } while (0);

  /*
   * IDENT_FIELD_ABSENT:
!  * Throw an error and exit the function if the given ident field ListCell is
   * not populated.
   *
   * IDENT_MULTI_VALUE:
!  * Throw an error and exit the function if the given ident token List has more
   * than one element.
   */
! #define IDENT_FIELD_ABSENT(field) do {\
      if (!field) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
--- 810,865 ----
   * MANDATORY_AUTH_ARG  = check if a required option is set for an authentication method,
   *                         reporting error if it's not.
   */
! #define INVALID_AUTH_OPTION(optname, validmethods) \
! do { \
!     ereport(elevel, \
              (errcode(ERRCODE_CONFIG_FILE_ERROR), \
               /* translator: the second %s is a list of auth methods */ \
               errmsg("authentication option \"%s\" is only valid for authentication methods %s", \
                      optname, _(validmethods)), \
               errcontext("line %d of configuration file \"%s\"", \
                      line_num, HbaFileName))); \
+     *err_msg = psprintf("authentication option \"%s\" is only valid for authentication methods %s", \
+                         optname, validmethods); \
      return false; \
! } while (0)

! #define REQUIRE_AUTH_OPTION(methodval, optname, validmethods) \
! do { \
      if (hbaline->auth_method != methodval) \
          INVALID_AUTH_OPTION(optname, validmethods); \
! } while (0)

! #define MANDATORY_AUTH_ARG(argvar, argname, authname) \
! do { \
!     if (argvar == NULL) { \
!         ereport(elevel, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
                   errmsg("authentication method \"%s\" requires argument \"%s\" to be set", \
                          authname, argname), \
                   errcontext("line %d of configuration file \"%s\"", \
                          line_num, HbaFileName))); \
+         *err_msg = psprintf("authentication method \"%s\" requires argument \"%s\" to be set", \
+                             authname, argname); \
          return NULL; \
      } \
! } while (0)

  /*
+  * Macros for handling pg_ident problems.
+  * Much as above, but currently the message level is hardwired as LOG
+  * and there is no provision for an err_msg string.
+  *
   * IDENT_FIELD_ABSENT:
!  * Log a message and exit the function if the given ident field ListCell is
   * not populated.
   *
   * IDENT_MULTI_VALUE:
!  * Log a message and exit the function if the given ident token List has more
   * than one element.
   */
! #define IDENT_FIELD_ABSENT(field) \
! do { \
      if (!field) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
*************** check_same_host_or_net(SockAddr *raddr,
*** 799,807 ****
                          IdentFileName, line_num))); \
          return NULL; \
      } \
! } while (0);

! #define IDENT_MULTI_VALUE(tokens) do {\
      if (tokens->length > 1) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
--- 867,876 ----
                          IdentFileName, line_num))); \
          return NULL; \
      } \
! } while (0)

! #define IDENT_MULTI_VALUE(tokens) \
! do { \
      if (tokens->length > 1) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
*************** check_same_host_or_net(SockAddr *raddr,
*** 810,832 ****
                              line_num, IdentFileName))); \
          return NULL; \
      } \
! } while (0);


  /*
   * Parse one tokenised line from the hba config file and store the result in a
   * HbaLine structure.
   *
!  * Return NULL if parsing fails.
   *
   * Note: this function leaks memory when an error occurs.  Caller is expected
   * to have set a memory context that will be reset if this function returns
   * NULL.
   */
  static HbaLine *
! parse_hba_line(TokenizedLine *tok_line)
  {
      int            line_num = tok_line->line_num;
      char       *str;
      struct addrinfo *gai_result;
      struct addrinfo hints;
--- 879,904 ----
                              line_num, IdentFileName))); \
          return NULL; \
      } \
! } while (0)


  /*
   * Parse one tokenised line from the hba config file and store the result in a
   * HbaLine structure.
   *
!  * If parsing fails, log a message at ereport level "elevel", store an error
!  * string in tok_line->err_msg, and return NULL.  (Some non-error conditions
!  * can also result in such messages.)
   *
   * Note: this function leaks memory when an error occurs.  Caller is expected
   * to have set a memory context that will be reset if this function returns
   * NULL.
   */
  static HbaLine *
! parse_hba_line(TokenizedLine *tok_line, int elevel)
  {
      int            line_num = tok_line->line_num;
+     char      **err_msg = &tok_line->err_msg;
      char       *str;
      struct addrinfo *gai_result;
      struct addrinfo hints;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 849,860 ****
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for connection type"),
                   errhint("Specify exactly one connection type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      token = linitial(tokens);
--- 921,933 ----
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for connection type"),
                   errhint("Specify exactly one connection type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "multiple values specified for connection type";
          return NULL;
      }
      token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 863,873 ****
  #ifdef HAVE_UNIX_SOCKETS
          parsedline->conntype = ctLocal;
  #else
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("local connections are not supported by this build"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
  #endif
      }
--- 936,947 ----
  #ifdef HAVE_UNIX_SOCKETS
          parsedline->conntype = ctLocal;
  #else
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("local connections are not supported by this build"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "local connections are not supported by this build";
          return NULL;
  #endif
      }
*************** parse_hba_line(TokenizedLine *tok_line)
*** 882,900 ****
              /* Log a warning if SSL support is not active */
  #ifdef USE_SSL
              if (!EnableSSL)
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                  errmsg("hostssl record cannot match because SSL is disabled"),
                           errhint("Set ssl = on in postgresql.conf."),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
  #else
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("hostssl record cannot match because SSL is not supported by this build"),
                errhint("Compile with --with-openssl to use SSL connections."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
  #endif
          }
          else if (token->string[4] == 'n')        /* "hostnossl" */
--- 956,978 ----
              /* Log a warning if SSL support is not active */
  #ifdef USE_SSL
              if (!EnableSSL)
!             {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                  errmsg("hostssl record cannot match because SSL is disabled"),
                           errhint("Set ssl = on in postgresql.conf."),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = "hostssl record cannot match because SSL is disabled";
+             }
  #else
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("hostssl record cannot match because SSL is not supported by this build"),
                errhint("Compile with --with-openssl to use SSL connections."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "hostssl record cannot match because SSL is not supported by this build";
  #endif
          }
          else if (token->string[4] == 'n')        /* "hostnossl" */
*************** parse_hba_line(TokenizedLine *tok_line)
*** 909,920 ****
      }                            /* record type */
      else
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid connection type \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 987,999 ----
      }                            /* record type */
      else
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid connection type \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("invalid connection type \"%s\"", token->string);
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 922,932 ****
      field = lnext(field);
      if (!field)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before database specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      parsedline->databases = NIL;
--- 1001,1012 ----
      field = lnext(field);
      if (!field)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before database specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "end-of-line before database specification";
          return NULL;
      }
      parsedline->databases = NIL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 941,951 ****
      field = lnext(field);
      if (!field)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before role specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      parsedline->roles = NIL;
--- 1021,1032 ----
      field = lnext(field);
      if (!field)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before role specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "end-of-line before role specification";
          return NULL;
      }
      parsedline->roles = NIL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 962,983 ****
          field = lnext(field);
          if (!field)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("end-of-line before IP address specification"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
          tokens = lfirst(field);
          if (tokens->length > 1)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("multiple values specified for host address"),
                       errhint("Specify one address range per line."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
          token = linitial(tokens);
--- 1043,1066 ----
          field = lnext(field);
          if (!field)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("end-of-line before IP address specification"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "end-of-line before IP address specification";
              return NULL;
          }
          tokens = lfirst(field);
          if (tokens->length > 1)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("multiple values specified for host address"),
                       errhint("Specify one address range per line."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "multiple values specified for host address";
              return NULL;
          }
          token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1027,1038 ****
                  parsedline->hostname = str;
              else
              {
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("invalid IP address \"%s\": %s",
                                  str, gai_strerror(ret)),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  if (gai_result)
                      pg_freeaddrinfo_all(hints.ai_family, gai_result);
                  return NULL;
--- 1110,1123 ----
                  parsedline->hostname = str;
              else
              {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("invalid IP address \"%s\": %s",
                                  str, gai_strerror(ret)),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = psprintf("invalid IP address \"%s\": %s",
+                                     str, gai_strerror(ret));
                  if (gai_result)
                      pg_freeaddrinfo_all(hints.ai_family, gai_result);
                  return NULL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1045,1068 ****
              {
                  if (parsedline->hostname)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("specifying both host name and CIDR mask is invalid: \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }

                  if (pg_sockaddr_cidr_mask(&parsedline->mask, cidr_slash + 1,
                                            parsedline->addr.ss_family) < 0)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid CIDR mask in address \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
                  pfree(str);
--- 1130,1157 ----
              {
                  if (parsedline->hostname)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("specifying both host name and CIDR mask is invalid: \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = psprintf("specifying both host name and CIDR mask is invalid: \"%s\"",
+                                         token->string);
                      return NULL;
                  }

                  if (pg_sockaddr_cidr_mask(&parsedline->mask, cidr_slash + 1,
                                            parsedline->addr.ss_family) < 0)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid CIDR mask in address \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = psprintf("invalid CIDR mask in address \"%s\"",
+                                         token->string);
                      return NULL;
                  }
                  pfree(str);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1074,1095 ****
                  field = lnext(field);
                  if (!field)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                            errmsg("end-of-line before netmask specification"),
                               errhint("Specify an address range in CIDR notation, or provide a separate netmask."),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
                  tokens = lfirst(field);
                  if (tokens->length > 1)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("multiple values specified for netmask"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
                  token = linitial(tokens);
--- 1163,1186 ----
                  field = lnext(field);
                  if (!field)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                            errmsg("end-of-line before netmask specification"),
                               errhint("Specify an address range in CIDR notation, or provide a separate netmask."),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = "end-of-line before netmask specification";
                      return NULL;
                  }
                  tokens = lfirst(field);
                  if (tokens->length > 1)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("multiple values specified for netmask"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = "multiple values specified for netmask";
                      return NULL;
                  }
                  token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1098,1109 ****
                                           &hints, &gai_result);
                  if (ret || !gai_result)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid IP mask \"%s\": %s",
                                      token->string, gai_strerror(ret)),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      if (gai_result)
                          pg_freeaddrinfo_all(hints.ai_family, gai_result);
                      return NULL;
--- 1189,1202 ----
                                           &hints, &gai_result);
                  if (ret || !gai_result)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid IP mask \"%s\": %s",
                                      token->string, gai_strerror(ret)),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = psprintf("invalid IP mask \"%s\": %s",
+                                         token->string, gai_strerror(ret));
                      if (gai_result)
                          pg_freeaddrinfo_all(hints.ai_family, gai_result);
                      return NULL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1115,1125 ****

                  if (parsedline->addr.ss_family != parsedline->mask.ss_family)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("IP address and mask do not match"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
              }
--- 1208,1219 ----

                  if (parsedline->addr.ss_family != parsedline->mask.ss_family)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("IP address and mask do not match"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = "IP address and mask do not match";
                      return NULL;
                  }
              }
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1130,1151 ****
      field = lnext(field);
      if (!field)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before authentication method"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for authentication type"),
                   errhint("Specify exactly one authentication type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      token = linitial(tokens);
--- 1224,1247 ----
      field = lnext(field);
      if (!field)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before authentication method"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "end-of-line before authentication method";
          return NULL;
      }
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for authentication type"),
                   errhint("Specify exactly one authentication type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "multiple values specified for authentication type";
          return NULL;
      }
      token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1177,1187 ****
      {
          if (Db_user_namespace)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
          parsedline->auth_method = uaMD5;
--- 1273,1284 ----
      {
          if (Db_user_namespace)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled";
              return NULL;
          }
          parsedline->auth_method = uaMD5;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1214,1236 ****
          parsedline->auth_method = uaRADIUS;
      else
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

      if (unsupauth)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\": not supported by this build",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1311,1337 ----
          parsedline->auth_method = uaRADIUS;
      else
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("invalid authentication method \"%s\"",
+                             token->string);
          return NULL;
      }

      if (unsupauth)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\": not supported by this build",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("invalid authentication method \"%s\": not supported by this build",
+                             token->string);
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 1246,1267 ****
      if (parsedline->conntype == ctLocal &&
          parsedline->auth_method == uaGSS)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
             errmsg("gssapi authentication is not supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

      if (parsedline->conntype != ctLocal &&
          parsedline->auth_method == uaPeer)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("peer authentication is only supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1347,1370 ----
      if (parsedline->conntype == ctLocal &&
          parsedline->auth_method == uaGSS)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
             errmsg("gssapi authentication is not supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "gssapi authentication is not supported on local sockets";
          return NULL;
      }

      if (parsedline->conntype != ctLocal &&
          parsedline->auth_method == uaPeer)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("peer authentication is only supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "peer authentication is only supported on local sockets";
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 1274,1284 ****
      if (parsedline->conntype != ctHostSSL &&
          parsedline->auth_method == uaCert)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("cert authentication is only supported on hostssl connections"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1377,1388 ----
      if (parsedline->conntype != ctHostSSL &&
          parsedline->auth_method == uaCert)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("cert authentication is only supported on hostssl connections"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "cert authentication is only supported on hostssl connections";
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 1323,1338 ****
                  /*
                   * Got something that's not a name=value pair.
                   */
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("authentication option not in name=value format: %s", token->string),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  return NULL;
              }

              *val++ = '\0';        /* str now holds "name", val holds "value" */
!             if (!parse_hba_auth_opt(str, val, parsedline, line_num))
                  /* parse_hba_auth_opt already logged the error message */
                  return NULL;
              pfree(str);
--- 1427,1444 ----
                  /*
                   * Got something that's not a name=value pair.
                   */
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("authentication option not in name=value format: %s", token->string),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = psprintf("authentication option not in name=value format: %s",
+                                     token->string);
                  return NULL;
              }

              *val++ = '\0';        /* str now holds "name", val holds "value" */
!             if (!parse_hba_auth_opt(str, val, parsedline, elevel, err_msg))
                  /* parse_hba_auth_opt already logged the error message */
                  return NULL;
              pfree(str);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1360,1380 ****
                  parsedline->ldapbindpasswd ||
                  parsedline->ldapsearchattribute)
              {
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl
togetherwith ldapprefix"), 
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  return NULL;
              }
          }
          else if (!parsedline->ldapbasedn)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or
\"ldapsuffix\"to be set"), 
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
      }
--- 1466,1488 ----
                  parsedline->ldapbindpasswd ||
                  parsedline->ldapsearchattribute)
              {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl
togetherwith ldapprefix"), 
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = "cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl
togetherwith ldapprefix"; 
                  return NULL;
              }
          }
          else if (!parsedline->ldapbasedn)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or
\"ldapsuffix\"to be set"), 
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or
\"ldapsuffix\"to be set"; 
              return NULL;
          }
      }
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1399,1409 ****
  /*
   * Parse one name-value pair as an authentication option into the given
   * HbaLine.  Return true if we successfully parse the option, false if we
!  * encounter an error.
   */
  static bool
! parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
  {
  #ifdef USE_LDAP
      hbaline->ldapscope = LDAP_SCOPE_SUBTREE;
  #endif
--- 1507,1521 ----
  /*
   * Parse one name-value pair as an authentication option into the given
   * HbaLine.  Return true if we successfully parse the option, false if we
!  * encounter an error.  In the event of an error, also log a message at
!  * ereport level "elevel", and store a message string into *err_msg.
   */
  static bool
! parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
!                    int elevel, char **err_msg)
  {
+     int            line_num = hbaline->linenumber;
+
  #ifdef USE_LDAP
      hbaline->ldapscope = LDAP_SCOPE_SUBTREE;
  #endif
*************** parse_hba_auth_opt(char *name, char *val
*** 1422,1432 ****
      {
          if (hbaline->conntype != ctHostSSL)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("clientcert can only be configured for \"hostssl\" rows"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return false;
          }
          if (strcmp(val, "1") == 0)
--- 1534,1545 ----
      {
          if (hbaline->conntype != ctHostSSL)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("clientcert can only be configured for \"hostssl\" rows"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "clientcert can only be configured for \"hostssl\" rows";
              return false;
          }
          if (strcmp(val, "1") == 0)
*************** parse_hba_auth_opt(char *name, char *val
*** 1437,1447 ****
          {
              if (hbaline->auth_method == uaCert)
              {
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("clientcert can not be set to 0 when using \"cert\" authentication"),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  return false;
              }
              hbaline->clientcert = false;
--- 1550,1561 ----
          {
              if (hbaline->auth_method == uaCert)
              {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("clientcert can not be set to 0 when using \"cert\" authentication"),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = "clientcert can not be set to 0 when using \"cert\" authentication";
                  return false;
              }
              hbaline->clientcert = false;
*************** parse_hba_auth_opt(char *name, char *val
*** 1473,1489 ****
          rc = ldap_url_parse(val, &urldata);
          if (rc != LDAP_SUCCESS)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not parse LDAP URL \"%s\": %s", val, ldap_err2string(rc))));
              return false;
          }

          if (strcmp(urldata->lud_scheme, "ldap") != 0)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("unsupported LDAP URL scheme: %s", urldata->lud_scheme)));
              ldap_free_urldesc(urldata);
              return false;
          }
--- 1587,1607 ----
          rc = ldap_url_parse(val, &urldata);
          if (rc != LDAP_SUCCESS)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not parse LDAP URL \"%s\": %s", val, ldap_err2string(rc))));
+             *err_msg = psprintf("could not parse LDAP URL \"%s\": %s",
+                                 val, ldap_err2string(rc));
              return false;
          }

          if (strcmp(urldata->lud_scheme, "ldap") != 0)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("unsupported LDAP URL scheme: %s", urldata->lud_scheme)));
+             *err_msg = psprintf("unsupported LDAP URL scheme: %s",
+                                 urldata->lud_scheme);
              ldap_free_urldesc(urldata);
              return false;
          }
*************** parse_hba_auth_opt(char *name, char *val
*** 1497,1513 ****
          hbaline->ldapscope = urldata->lud_scope;
          if (urldata->lud_filter)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("filters not supported in LDAP URLs")));
              ldap_free_urldesc(urldata);
              return false;
          }
          ldap_free_urldesc(urldata);
  #else                            /* not OpenLDAP */
!         ereport(LOG,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                   errmsg("LDAP URLs not supported on this platform")));
  #endif   /* not OpenLDAP */
      }
      else if (strcmp(name, "ldaptls") == 0)
--- 1615,1633 ----
          hbaline->ldapscope = urldata->lud_scope;
          if (urldata->lud_filter)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("filters not supported in LDAP URLs")));
+             *err_msg = "filters not supported in LDAP URLs";
              ldap_free_urldesc(urldata);
              return false;
          }
          ldap_free_urldesc(urldata);
  #else                            /* not OpenLDAP */
!         ereport(elevel,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                   errmsg("LDAP URLs not supported on this platform")));
+         *err_msg = "LDAP URLs not supported on this platform";
  #endif   /* not OpenLDAP */
      }
      else if (strcmp(name, "ldaptls") == 0)
*************** parse_hba_auth_opt(char *name, char *val
*** 1529,1539 ****
          hbaline->ldapport = atoi(val);
          if (hbaline->ldapport == 0)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid LDAP port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return false;
          }
      }
--- 1649,1660 ----
          hbaline->ldapport = atoi(val);
          if (hbaline->ldapport == 0)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid LDAP port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = psprintf("invalid LDAP port number: \"%s\"", val);
              return false;
          }
      }
*************** parse_hba_auth_opt(char *name, char *val
*** 1617,1628 ****
          ret = pg_getaddrinfo_all(val, NULL, &hints, &gai_result);
          if (ret || !gai_result)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not translate RADIUS server name \"%s\" to address: %s",
                              val, gai_strerror(ret)),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              if (gai_result)
                  pg_freeaddrinfo_all(hints.ai_family, gai_result);
              return false;
--- 1738,1751 ----
          ret = pg_getaddrinfo_all(val, NULL, &hints, &gai_result);
          if (ret || !gai_result)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not translate RADIUS server name \"%s\" to address: %s",
                              val, gai_strerror(ret)),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = psprintf("could not translate RADIUS server name \"%s\" to address: %s",
+                                 val, gai_strerror(ret));
              if (gai_result)
                  pg_freeaddrinfo_all(hints.ai_family, gai_result);
              return false;
*************** parse_hba_auth_opt(char *name, char *val
*** 1636,1646 ****
          hbaline->radiusport = atoi(val);
          if (hbaline->radiusport == 0)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid RADIUS port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return false;
          }
      }
--- 1759,1770 ----
          hbaline->radiusport = atoi(val);
          if (hbaline->radiusport == 0)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid RADIUS port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = psprintf("invalid RADIUS port number: \"%s\"", val);
              return false;
          }
      }
*************** parse_hba_auth_opt(char *name, char *val
*** 1656,1667 ****
      }
      else
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("unrecognized authentication option name: \"%s\"",
                          name),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return false;
      }
      return true;
--- 1780,1793 ----
      }
      else
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("unrecognized authentication option name: \"%s\"",
                          name),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("unrecognized authentication option name: \"%s\"",
+                             name);
          return false;
      }
      return true;
*************** load_hba(void)
*** 1794,1800 ****
          return false;
      }

!     linecxt = tokenize_file(HbaFileName, file, &hba_lines);
      FreeFile(file);

      /* Now parse all the lines */
--- 1920,1926 ----
          return false;
      }

!     linecxt = tokenize_file(HbaFileName, file, &hba_lines, LOG);
      FreeFile(file);

      /* Now parse all the lines */
*************** load_hba(void)
*** 1808,1828 ****
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line);
          HbaLine    *newline;

!         if ((newline = parse_hba_line(tok_line)) == NULL)
          {
!             /*
!              * Parse error in the file, so indicate there's a problem.  NB: a
!              * problem in a line will free the memory for all previous lines
!              * as well!
!              */
!             MemoryContextReset(hbacxt);
!             new_parsed_lines = NIL;
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first row. Error has already been reported in the
!              * parsing function, so no need to log it here.
               */
              continue;
          }
--- 1934,1955 ----
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line);
          HbaLine    *newline;

!         /* don't parse lines that already have errors */
!         if (tok_line->err_msg != NULL)
          {
!             ok = false;
!             continue;
!         }
!
!         if ((newline = parse_hba_line(tok_line, LOG)) == NULL)
!         {
!             /* Parse error; remember there's trouble */
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first line.  Error has already been logged, no
!              * need for more chatter here.
               */
              continue;
          }
*************** load_hba(void)
*** 1865,1874 ****
  }

  /*
   * Parse one tokenised line from the ident config file and store the result in
   * an IdentLine structure.
   *
!  * Return NULL if parsing fails.
   *
   * If ident_user is a regular expression (ie. begins with a slash), it is
   * compiled and stored in IdentLine structure.
--- 1992,2393 ----
  }

  /*
+  * This macro specifies the maximum number of authentication options
+  * that are possible with any given authentication method that is supported.
+  * Currently LDAP supports 10, so the macro value is well above the most any
+  * method needs.
+  */
+ #define MAX_HBA_OPTIONS 12
+
+ /*
+  * Create a text array listing the options specified in the HBA line.
+  * Return NULL if no options are specified.
+  */
+ static ArrayType *
+ gethba_options(HbaLine *hba)
+ {
+     int            noptions;
+     Datum        options[MAX_HBA_OPTIONS];
+
+     noptions = 0;
+
+     if (hba->auth_method == uaGSS || hba->auth_method == uaSSPI)
+     {
+         if (hba->include_realm)
+             options[noptions++] =
+                 CStringGetTextDatum("include_realm=true");
+
+         if (hba->krb_realm)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("krb_realm=%s", hba->krb_realm));
+     }
+
+     if (hba->usermap)
+         options[noptions++] =
+             CStringGetTextDatum(psprintf("map=%s", hba->usermap));
+
+     if (hba->clientcert)
+         options[noptions++] =
+             CStringGetTextDatum("clientcert=true");
+
+     if (hba->pamservice)
+         options[noptions++] =
+             CStringGetTextDatum(psprintf("pamservice=%s", hba->pamservice));
+
+     if (hba->auth_method == uaLDAP)
+     {
+         if (hba->ldapserver)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapserver=%s", hba->ldapserver));
+
+         if (hba->ldapport)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapport=%d", hba->ldapport));
+
+         if (hba->ldaptls)
+             options[noptions++] =
+                 CStringGetTextDatum("ldaptls=true");
+
+         if (hba->ldapprefix)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapprefix=%s", hba->ldapprefix));
+
+         if (hba->ldapsuffix)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapsuffix=%s", hba->ldapsuffix));
+
+         if (hba->ldapbasedn)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapbasedn=%s", hba->ldapbasedn));
+
+         if (hba->ldapbinddn)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapbinddn=%s", hba->ldapbinddn));
+
+         if (hba->ldapbindpasswd)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapbindpasswd=%s",
+                                              hba->ldapbindpasswd));
+
+         if (hba->ldapsearchattribute)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapsearchattribute=%s",
+                                              hba->ldapsearchattribute));
+
+         if (hba->ldapscope)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapscope=%d", hba->ldapscope));
+     }
+
+     if (hba->auth_method == uaRADIUS)
+     {
+         if (hba->radiusserver)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiusserver=%s", hba->radiusserver));
+
+         if (hba->radiussecret)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiussecret=%s", hba->radiussecret));
+
+         if (hba->radiusidentifier)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiusidentifier=%s", hba->radiusidentifier));
+
+         if (hba->radiusport)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiusport=%d", hba->radiusport));
+     }
+
+     Assert(noptions <= MAX_HBA_OPTIONS);
+
+     if (noptions > 0)
+         return construct_array(options, noptions, TEXTOID, -1, false, 'i');
+     else
+         return NULL;
+ }
+
+ /* Number of columns in pg_hba_rules view */
+ #define NUM_PG_HBA_RULES_ATTS     9
+
+ /*
+  * fill_hba_line: construct one row of pg_hba_rules view, add it to tuplestore
+  *
+  * tuple_store: where to store data
+  * tupdesc: tuple descriptor for the view
+  * lineno: pg_hba line number (must always be valid)
+  * hba: parsed line data (can be NULL, in which case err_msg should be set)
+  * err_msg: error message (NULL if none)
+  *
+  * Note: leaks memory, but we don't care since this is run in a short-lived
+  * memory context.
+  */
+ static void
+ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
+               int lineno, HbaLine *hba, const char *err_msg)
+ {
+     Datum        values[NUM_PG_HBA_RULES_ATTS];
+     bool        nulls[NUM_PG_HBA_RULES_ATTS];
+     char        buffer[NI_MAXHOST];
+     HeapTuple    tuple;
+     int            index;
+     ListCell   *lc;
+     ArrayType  *options;
+
+     memset(values, 0, sizeof(values));
+     memset(nulls, 0, sizeof(nulls));
+     index = 0;
+
+     /* line_number */
+     values[index++] = Int32GetDatum(lineno);
+
+     if (hba != NULL)
+     {
+         /* type */
+         switch (hba->conntype)
+         {
+             case ctLocal:
+                 values[index++] = CStringGetTextDatum("local");
+                 break;
+             case ctHost:
+                 values[index++] = CStringGetTextDatum("host");
+                 break;
+             case ctHostSSL:
+                 values[index++] = CStringGetTextDatum("hostssl");
+                 break;
+             case ctHostNoSSL:
+                 values[index++] = CStringGetTextDatum("hostnossl");
+                 break;
+             default:
+                 elog(ERROR, "unrecognized conntype: %d", (int) hba->conntype);
+                 break;
+         }
+
+         /* database */
+         if (hba->databases)
+         {
+             /* flatten HbaToken list to string list */
+             List       *names = NIL;
+
+             foreach(lc, hba->databases)
+             {
+                 HbaToken   *tok = lfirst(lc);
+
+                 names = lappend(names, tok->string);
+             }
+             values[index++] = PointerGetDatum(strlist_to_textarray(names));
+         }
+         else
+             nulls[index++] = true;
+
+         /* user */
+         if (hba->roles)
+         {
+             /* flatten HbaToken list to string list */
+             List       *roles = NIL;
+
+             foreach(lc, hba->roles)
+             {
+                 HbaToken   *tok = lfirst(lc);
+
+                 roles = lappend(roles, tok->string);
+             }
+             values[index++] = PointerGetDatum(strlist_to_textarray(roles));
+         }
+         else
+             nulls[index++] = true;
+
+         /* address and netmask */
+         switch (hba->ip_cmp_method)
+         {
+             case ipCmpMask:
+                 if (hba->hostname)
+                 {
+                     values[index++] = CStringGetTextDatum(hba->hostname);
+                     nulls[index++] = true;
+                 }
+                 else
+                 {
+                     if (pg_getnameinfo_all(&hba->addr, sizeof(hba->addr),
+                                            buffer, sizeof(buffer),
+                                            NULL, 0,
+                                            NI_NUMERICHOST) == 0)
+                     {
+                         clean_ipv6_addr(hba->addr.ss_family, buffer);
+                         values[index++] = CStringGetTextDatum(buffer);
+                     }
+                     else
+                         nulls[index++] = true;
+
+                     /* netmask */
+                     if (pg_getnameinfo_all(&hba->mask, sizeof(hba->mask),
+                                            buffer, sizeof(buffer),
+                                            NULL, 0,
+                                            NI_NUMERICHOST) == 0)
+                     {
+                         clean_ipv6_addr(hba->mask.ss_family, buffer);
+                         values[index++] = CStringGetTextDatum(buffer);
+                     }
+                     else
+                         nulls[index++] = true;
+                 }
+                 break;
+             case ipCmpAll:
+                 values[index++] = CStringGetTextDatum("all");
+                 nulls[index++] = true;
+                 break;
+             case ipCmpSameHost:
+                 values[index++] = CStringGetTextDatum("samehost");
+                 nulls[index++] = true;
+                 break;
+             case ipCmpSameNet:
+                 values[index++] = CStringGetTextDatum("samenet");
+                 nulls[index++] = true;
+                 break;
+             default:
+                 elog(ERROR, "unrecognized ip_cmp_method: %d",
+                      (int) hba->ip_cmp_method);
+                 break;
+         }
+
+         /* auth_method */
+         values[index++] = CStringGetTextDatum(UserAuthName[hba->auth_method]);
+
+         /* options */
+         options = gethba_options(hba);
+         if (options)
+             values[index++] = PointerGetDatum(options);
+         else
+             nulls[index++] = true;
+     }
+     else
+     {
+         /* no parsing result, so set relevant fields to nulls */
+         memset(&nulls[1], true, (NUM_PG_HBA_RULES_ATTS - 2) * sizeof(bool));
+     }
+
+     /* error */
+     if (err_msg)
+         values[NUM_PG_HBA_RULES_ATTS - 1] = CStringGetTextDatum(err_msg);
+     else
+         nulls[NUM_PG_HBA_RULES_ATTS - 1] = true;
+
+     tuple = heap_form_tuple(tupdesc, values, nulls);
+     tuplestore_puttuple(tuple_store, tuple);
+ }
+
+ /*
+  * Read the config file and fill the tuplestore with view records.
+  */
+ static void
+ fill_hba(Tuplestorestate *tuple_store, TupleDesc tupdesc)
+ {
+     FILE       *file;
+     List       *hba_lines = NIL;
+     ListCell   *line;
+     MemoryContext linecxt;
+     MemoryContext hbacxt;
+     MemoryContext oldcxt;
+
+     /*
+      * In the unlikely event that we can't open pg_hba.conf, we throw an
+      * error, rather than trying to report it via some sort of view entry.
+      * (Most other error conditions should result in a message in a view
+      * entry.)
+      */
+     file = AllocateFile(HbaFileName, "r");
+     if (file == NULL)
+         ereport(ERROR,
+                 (errcode_for_file_access(),
+                  errmsg("could not open configuration file \"%s\": %m",
+                         HbaFileName)));
+
+     linecxt = tokenize_file(HbaFileName, file, &hba_lines, DEBUG3);
+     FreeFile(file);
+
+     /* Now parse all the lines */
+     hbacxt = AllocSetContextCreate(CurrentMemoryContext,
+                                    "hba parser context",
+                                    ALLOCSET_SMALL_SIZES);
+     oldcxt = MemoryContextSwitchTo(hbacxt);
+     foreach(line, hba_lines)
+     {
+         TokenizedLine *tok_line = (TokenizedLine *) lfirst(line);
+         HbaLine    *hbaline = NULL;
+
+         /* don't parse lines that already have errors */
+         if (tok_line->err_msg == NULL)
+             hbaline = parse_hba_line(tok_line, DEBUG3);
+
+         fill_hba_line(tuple_store, tupdesc, tok_line->line_num,
+                       hbaline, tok_line->err_msg);
+     }
+
+     /* Free tokenizer memory */
+     MemoryContextDelete(linecxt);
+     /* Free parse_hba_line memory */
+     MemoryContextSwitchTo(oldcxt);
+     MemoryContextDelete(hbacxt);
+ }
+
+ /*
+  * SQL-accessible SRF to return all the entries from the pg_hba.conf file.
+  */
+ Datum
+ pg_hba_rules(PG_FUNCTION_ARGS)
+ {
+     Tuplestorestate *tuple_store;
+     TupleDesc    tupdesc;
+     MemoryContext old_cxt;
+     ReturnSetInfo *rsi;
+
+     /*
+      * We must use the Materialize mode to be safe against HBA file reloads
+      * while the cursor is open. It's also more efficient than having to look
+      * up our current position in the parsed list every time.
+      */
+     rsi = (ReturnSetInfo *) fcinfo->resultinfo;
+
+     /* Check to see if caller supports us returning a tuplestore */
+     if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("set-valued function called in context that cannot accept a set")));
+     if (!(rsi->allowedModes & SFRM_Materialize))
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("materialize mode required, but it is not " \
+                         "allowed in this context")));
+
+     rsi->returnMode = SFRM_Materialize;
+
+     /* Build a tuple descriptor for our result type */
+     if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+         elog(ERROR, "return type must be a row type");
+     Assert(tupdesc->natts == NUM_PG_HBA_RULES_ATTS);
+
+     /* Build tuplestore to hold the result rows */
+     old_cxt = MemoryContextSwitchTo(rsi->econtext->ecxt_per_query_memory);
+
+     tuple_store =
+         tuplestore_begin_heap(rsi->allowedModes & SFRM_Materialize_Random,
+                               false, work_mem);
+     rsi->setDesc = tupdesc;
+     rsi->setResult = tuple_store;
+
+     MemoryContextSwitchTo(old_cxt);
+
+     /* Fill the tuplestore */
+     fill_hba(tuple_store, tupdesc);
+
+     PG_RETURN_NULL();
+ }
+
+
+ /*
   * Parse one tokenised line from the ident config file and store the result in
   * an IdentLine structure.
   *
!  * If parsing fails, log a message and return NULL.
   *
   * If ident_user is a regular expression (ie. begins with a slash), it is
   * compiled and stored in IdentLine structure.
*************** load_ident(void)
*** 2170,2176 ****
          return false;
      }

!     linecxt = tokenize_file(IdentFileName, file, &ident_lines);
      FreeFile(file);

      /* Now parse all the lines */
--- 2689,2695 ----
          return false;
      }

!     linecxt = tokenize_file(IdentFileName, file, &ident_lines, LOG);
      FreeFile(file);

      /* Now parse all the lines */
*************** load_ident(void)
*** 2183,2208 ****
      {
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line_cell);

          if ((newline = parse_ident_line(tok_line)) == NULL)
          {
!             /*
!              * Parse error in the file, so indicate there's a problem.  Free
!              * all the memory and regular expressions of lines parsed so far.
!              */
!             foreach(parsed_line_cell, new_parsed_lines)
!             {
!                 newline = (IdentLine *) lfirst(parsed_line_cell);
!                 if (newline->ident_user[0] == '/')
!                     pg_regfree(&newline->re);
!             }
!             MemoryContextReset(ident_context);
!             new_parsed_lines = NIL;
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first row. Error has already been reported in the
!              * parsing function, so no need to log it here.
               */
              continue;
          }
--- 2702,2723 ----
      {
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line_cell);

+         /* don't parse lines that already have errors */
+         if (tok_line->err_msg != NULL)
+         {
+             ok = false;
+             continue;
+         }
+
          if ((newline = parse_ident_line(tok_line)) == NULL)
          {
!             /* Parse error; remember there's trouble */
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first line.  Error has already been logged, no
!              * need for more chatter here.
               */
              continue;
          }
*************** load_ident(void)
*** 2216,2222 ****

      if (!ok)
      {
!         /* File contained one or more errors, so bail out */
          foreach(parsed_line_cell, new_parsed_lines)
          {
              newline = (IdentLine *) lfirst(parsed_line_cell);
--- 2731,2741 ----

      if (!ok)
      {
!         /*
!          * File contained one or more errors, so bail out, first being careful
!          * to clean up whatever we allocated.  Most stuff will go away via
!          * MemoryContextDelete, but we have to clean up regexes explicitly.
!          */
          foreach(parsed_line_cell, new_parsed_lines)
          {
              newline = (IdentLine *) lfirst(parsed_line_cell);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 31c828a..dd1568d 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DATA(insert OID = 2084 (  pg_show_all_se
*** 3076,3081 ****
--- 3076,3083 ----
  DESCR("SHOW ALL as a function");
  DATA(insert OID = 3329 (  pg_show_all_file_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t v s 0 0 2249 ""
"{25,23,23,25,25,16,25}""{o,o,o,o,o,o,o}" "{sourcefile,sourceline,seqno,name,setting,applied,error}" _null_ _null_
show_all_file_settings_null_ _null_ _null_ )); 
  DESCR("show config file settings");
+ DATA(insert OID = 3401 (  pg_hba_rules PGNSP PGUID 12 1 1000 0 0 f f f f t t v s 0 0 2249 ""
"{23,25,1009,1009,25,25,25,1009,25}""{o,o,o,o,o,o,o,o,o}"
"{line_number,type,database,user_name,address,netmask,auth_method,options,error}"_null_ _null_ pg_hba_rules _null_
_null__null_ )); 
+ DESCR("show pg_hba config rules");
  DATA(insert OID = 1371 (  pg_lock_status   PGNSP PGUID 12 1 1000 0 0 f f f f t t v s 0 0 2249 ""
"{25,26,26,23,21,25,28,26,26,21,25,23,25,16,16}""{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}"
"{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted,fastpath}"
_null__null_ pg_lock_status _null_ _null_ _null_ )); 
  DESCR("view system lock information");
  DATA(insert OID = 2561 (  pg_blocking_pids PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 1007 "23" _null_ _null_ _null_
_null__null_ pg_blocking_pids _null_ _null_ _null_ )); 
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index dc7d257..893767f 100644
*** a/src/include/libpq/hba.h
--- b/src/include/libpq/hba.h
***************
*** 16,25 ****
  #include "regex/regex.h"


  typedef enum UserAuth
  {
      uaReject,
!     uaImplicitReject,
      uaTrust,
      uaIdent,
      uaPassword,
--- 16,31 ----
  #include "regex/regex.h"


+ /*
+  * The following enum represents the authentication methods that
+  * are supported by PostgreSQL.
+  *
+  * Note: keep this in sync with the UserAuthName array in hba.c.
+  */
  typedef enum UserAuth
  {
      uaReject,
!     uaImplicitReject,            /* Not a user-visible option */
      uaTrust,
      uaIdent,
      uaPassword,
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 60abcad..de7860a 100644
*** a/src/test/regress/expected/rules.out
--- b/src/test/regress/expected/rules.out
*************** pg_group| SELECT pg_authid.rolname AS gr
*** 1338,1343 ****
--- 1338,1353 ----
            WHERE (pg_auth_members.roleid = pg_authid.oid)) AS grolist
     FROM pg_authid
    WHERE (NOT pg_authid.rolcanlogin);
+ pg_hba_rules| SELECT a.line_number,
+     a.type,
+     a.database,
+     a.user_name,
+     a.address,
+     a.netmask,
+     a.auth_method,
+     a.options,
+     a.error
+    FROM pg_hba_rules() a(line_number, type, database, user_name, address, netmask, auth_method, options, error);
  pg_indexes| SELECT n.nspname AS schemaname,
      c.relname AS tablename,
      i.relname AS indexname,

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

I wrote:
> I'm still not very happy about the choice of view name ...

After looking over this thread again, I think that we should go with
pg_file_hba_rules or perhaps pg_hba_file_rules.  I see that options
like that were discussed and rejected earlier, but I feel the arguments
against were based on false premises.  I think we need "file" in the
name because:

1. It makes the analogy to the pg_file_settings view clearer.

2. It emphasizes that what you see in the view is the contents of
the disk files, not necessarily the active rules.

3. It leaves the door open to use "pg_hba_rules" as the name of some
future view that *does* show the active rules, analogously to pg_settings
which does show the active GUC settings.

I realize that there's no very convenient way to implement a true
active-auth-rules view right now, but it's not hard to see how that
could be fixed if we were motivated to do so.  One simple way would
be for the postmaster, any time it had successfully loaded the hba
file, to write out some representation of the parsed data into an
"active auth rules" file.  I doubt anyone would bother if the only
application were an active-rules view, but there's at least one
other reason to do this, which is that we could make the HBA stuff
work the same on Windows as it does elsewhere.  Right now, since
new EXEC_BACKEND backends must read the HBA files for themselves,
Windows does not have the property that pg_hba.conf is read only
at SIGHUP --- break the file with some fat-fingered editing, and
new connections begin to fail instantly.  But if new backends
always read a postmaster-written file, then the behavior would be
the same as it is on Unix.
        regards, tom lane

I wrote:
> I spent awhile hacking on this, and made a lot of things better, but
> I'm still very unhappy about the state of the comments.

I made another pass over this, working on the comments and the docs,
and changing the view name to "pg_hba_file_rules".  I think this version
is committable if people are satisfied with that name.

One loose end is what to do about testing.  I did not much like the
proposed TAP tests.  We could just put "select count(*) > 0 from
pg_hba_file_rules" into the main regression tests, which would provide
some code coverage there, if not very much guarantee that what the view
outputs is sane.

            regards, tom lane

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 086fafc..204b8cf 100644
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***************
*** 7809,7814 ****
--- 7809,7819 ----
       </row>

       <row>
+       <entry><link linkend="view-pg-hba-file-rules"><structname>pg_hba_file_rules</structname></link></entry>
+       <entry>summary of client authentication configuration file contents</entry>
+      </row>
+
+      <row>
        <entry><link linkend="view-pg-indexes"><structname>pg_indexes</structname></link></entry>
        <entry>indexes</entry>
       </row>
***************
*** 8408,8413 ****
--- 8413,8526 ----

   </sect1>

+  <sect1 id="view-pg-hba-file-rules">
+   <title><structname>pg_hba_file_rules</structname></title>
+
+   <indexterm zone="view-pg-hba-file-rules">
+    <primary>pg_hba_file_rules</primary>
+   </indexterm>
+
+   <para>
+    The view <structname>pg_hba_file_rules</structname> provides a summary of
+    the contents of the client authentication configuration
+    file, <filename>pg_hba.conf</>.  A row appears in this view for each
+    non-empty, non-comment line in the file, with annotations indicating
+    whether the rule could be applied successfully.
+   </para>
+
+   <para>
+    This view can be helpful for checking whether planned changes in the
+    authentication configuration file will work, or for diagnosing a previous
+    failure.  Note that this view reports on the <emphasis>current</> contents
+    of the file, not on what was last loaded by the server.
+   </para>
+
+   <para>
+    By default, the <structname>pg_hba_file_rules</structname> view can be read
+    only by superusers.
+   </para>
+
+   <table>
+    <title><structname>pg_hba_file_rules</> Columns</title>
+
+   <tgroup cols="3">
+    <thead>
+     <row>
+      <entry>Name</entry>
+      <entry>Type</entry>
+      <entry>Description</entry>
+     </row>
+    </thead>
+    <tbody>
+     <row>
+      <entry><structfield>line_number</structfield></entry>
+      <entry><structfield>integer</structfield></entry>
+      <entry>
+       Line number of this rule in <filename>pg_hba.conf</>
+      </entry>
+     </row>
+     <row>
+      <entry><structfield>type</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>Type of connection</entry>
+     </row>
+     <row>
+      <entry><structfield>database</structfield></entry>
+      <entry><structfield>text[]</structfield></entry>
+      <entry>List of database name(s) to which this rule applies</entry>
+     </row>
+     <row>
+      <entry><structfield>user_name</structfield></entry>
+      <entry><structfield>text[]</structfield></entry>
+      <entry>List of user and group name(s) to which this rule applies</entry>
+     </row>
+     <row>
+      <entry><structfield>address</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>
+       Host name or IP address, or one
+       of <literal>all</literal>, <literal>samehost</literal>,
+       or <literal>samenet</literal>, or null for local connections
+      </entry>
+     </row>
+     <row>
+      <entry><structfield>netmask</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>IP address mask, or null if not applicable</entry>
+     </row>
+     <row>
+      <entry><structfield>auth_method</structfield></entry>
+      <entry><type>text</type></entry>
+      <entry>Authentication method</entry>
+     </row>
+     <row>
+      <entry><structfield>options</structfield></entry>
+      <entry><type>text[]</type></entry>
+      <entry>Options specified for authentication method, if any</entry>
+     </row>
+     <row>
+      <entry><structfield>error</structfield></entry>
+      <entry><structfield>text</structfield></entry>
+      <entry>
+       If not null, an error message indicating why this
+       line could not be processed
+      </entry>
+     </row>
+    </tbody>
+   </tgroup>
+   </table>
+
+   <para>
+    Usually, a row reflecting an incorrect entry will have values for only
+    the <structfield>line_number</> and <structfield>error</> fields.
+   </para>
+
+   <para>
+    See <xref linkend="client-authentication"> for more information about
+    client authentication configuration.
+   </para>
+  </sect1>
+
   <sect1 id="view-pg-indexes">
    <title><structname>pg_indexes</structname></title>

diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index dda5891..231fc40 100644
*** a/doc/src/sgml/client-auth.sgml
--- b/doc/src/sgml/client-auth.sgml
*************** hostnossl  <replaceable>database</replac
*** 597,602 ****
--- 597,620 ----
     re-read the file.
    </para>

+   <note>
+    <para>
+     The preceding statement is not true on Microsoft Windows: there, any
+     changes in the <filename>pg_hba.conf</filename> file are immediately
+     applied by subsequent new connections.
+    </para>
+   </note>
+
+   <para>
+    The system view
+    <link linkend="view-pg-hba-file-rules"><structname>pg_hba_file_rules</structname></link>
+    can be helpful for pre-testing changes to the <filename>pg_hba.conf</>
+    file, or for diagnosing problems if loading of the file did not have the
+    desired effects.  Rows in the view with
+    non-null <structfield>error</structfield> fields indicate problems in the
+    corresponding lines of the file.
+   </para>
+
    <tip>
     <para>
      To connect to a particular database, a user must not only pass the
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 4dfedf8..28be27a 100644
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*************** CREATE VIEW pg_file_settings AS
*** 459,464 ****
--- 459,470 ----
  REVOKE ALL on pg_file_settings FROM PUBLIC;
  REVOKE EXECUTE ON FUNCTION pg_show_all_file_settings() FROM PUBLIC;

+ CREATE VIEW pg_hba_file_rules AS
+    SELECT * FROM pg_hba_file_rules() AS A;
+
+ REVOKE ALL on pg_hba_file_rules FROM PUBLIC;
+ REVOKE EXECUTE ON FUNCTION pg_hba_file_rules() FROM PUBLIC;
+
  CREATE VIEW pg_timezone_abbrevs AS
      SELECT * FROM pg_timezone_abbrevs();

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index bbe0a88..7a0f1ce 100644
*** a/src/backend/libpq/hba.c
--- b/src/backend/libpq/hba.c
***************
*** 25,39 ****
--- 25,44 ----
  #include <arpa/inet.h>
  #include <unistd.h>

+ #include "access/htup_details.h"
  #include "catalog/pg_collation.h"
+ #include "catalog/pg_type.h"
  #include "common/ip.h"
+ #include "funcapi.h"
  #include "libpq/ifaddr.h"
  #include "libpq/libpq.h"
+ #include "miscadmin.h"
  #include "postmaster/postmaster.h"
  #include "regex/regex.h"
  #include "replication/walsender.h"
  #include "storage/fd.h"
  #include "utils/acl.h"
+ #include "utils/builtins.h"
  #include "utils/guc.h"
  #include "utils/lsyscache.h"
  #include "utils/memutils.h"
*************** typedef struct HbaToken
*** 80,91 ****
--- 85,99 ----
   * Each item in the "fields" list is a sub-list of HbaTokens.
   * We don't emit a TokenizedLine for empty or all-comment lines,
   * so "fields" is never NIL (nor are any of its sub-lists).
+  * Exception: if an error occurs during tokenization, we might
+  * have fields == NIL, in which case err_msg != NULL.
   */
  typedef struct TokenizedLine
  {
      List       *fields;            /* List of lists of HbaTokens */
      int            line_num;        /* Line number */
      char       *raw_line;        /* Raw line text */
+     char       *err_msg;        /* Error message if any */
  } TokenizedLine;

  /*
*************** static MemoryContext parsed_hba_context
*** 106,118 ****
  static List *parsed_ident_lines = NIL;
  static MemoryContext parsed_ident_context = NULL;


  static MemoryContext tokenize_file(const char *filename, FILE *file,
!               List **tok_lines);
  static List *tokenize_inc_file(List *tokens, const char *outer_filename,
!                   const char *inc_filename);
  static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
!                    int line_num);

  /*
   * isblank() exists in the ISO C99 spec, but it's not very portable yet,
--- 114,155 ----
  static List *parsed_ident_lines = NIL;
  static MemoryContext parsed_ident_context = NULL;

+ /*
+  * The following character array represents the names of the authentication
+  * methods that are supported by PostgreSQL.
+  *
+  * Note: keep this in sync with the UserAuth enum in hba.h.
+  */
+ static const char *const UserAuthName[] =
+ {
+     "reject",
+     "implicit reject",            /* Not a user-visible option */
+     "trust",
+     "ident",
+     "password",
+     "md5",
+     "gss",
+     "sspi",
+     "pam",
+     "bsd",
+     "ldap",
+     "cert",
+     "radius",
+     "peer"
+ };
+

  static MemoryContext tokenize_file(const char *filename, FILE *file,
!               List **tok_lines, int elevel);
  static List *tokenize_inc_file(List *tokens, const char *outer_filename,
!                   const char *inc_filename, int elevel, char **err_msg);
  static bool parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
!                    int elevel, char **err_msg);
! static ArrayType *gethba_options(HbaLine *hba);
! static void fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
!               int lineno, HbaLine *hba, const char *err_msg);
! static void fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc);
!

  /*
   * isblank() exists in the ISO C99 spec, but it's not very portable yet,
*************** pg_isblank(const char c)
*** 126,157 ****


  /*
!  * Grab one token out of the string pointed to by lineptr.
   * Tokens are strings of non-blank
   * characters bounded by blank characters, commas, beginning of line, and
   * end of line. Blank means space or tab. Tokens can be delimited by
   * double quotes (this allows the inclusion of blanks, but not newlines).
   *
-  * The token, if any, is returned at *buf (a buffer of size bufsz).
   * Also, we set *initial_quote to indicate whether there was quoting before
   * the first character.  (We use that to prevent "@x" from being treated
   * as a file inclusion request.  Note that @"x" should be so treated;
   * we want to allow that to support embedded spaces in file paths.)
   * We set *terminating_comma to indicate whether the token is terminated by a
!  * comma (which is not returned.)
   *
   * If successful: store null-terminated token at *buf and return TRUE.
   * If no more tokens on line: set *buf = '\0' and return FALSE.
!  *
!  * Leave file positioned at the character immediately after the token or EOF,
!  * whichever comes first. If no more tokens on line, position the file to the
!  * beginning of the next line or EOF, whichever comes first.
!  *
!  * Handle comments.
   */
  static bool
! next_token(char **lineptr, char *buf, int bufsz, bool *initial_quote,
!            bool *terminating_comma)
  {
      int            c;
      char       *start_buf = buf;
--- 163,199 ----


  /*
!  * Grab one token out of the string pointed to by *lineptr.
!  *
   * Tokens are strings of non-blank
   * characters bounded by blank characters, commas, beginning of line, and
   * end of line. Blank means space or tab. Tokens can be delimited by
   * double quotes (this allows the inclusion of blanks, but not newlines).
+  * Comments (started by an unquoted '#') are skipped.
+  *
+  * The token, if any, is returned at *buf (a buffer of size bufsz), and
+  * *lineptr is advanced past the token.
   *
   * Also, we set *initial_quote to indicate whether there was quoting before
   * the first character.  (We use that to prevent "@x" from being treated
   * as a file inclusion request.  Note that @"x" should be so treated;
   * we want to allow that to support embedded spaces in file paths.)
+  *
   * We set *terminating_comma to indicate whether the token is terminated by a
!  * comma (which is not returned).
!  *
!  * In event of an error, log a message at ereport level elevel, and also
!  * set *err_msg to a string describing the error.  Currently the only
!  * possible error is token too long for buf.
   *
   * If successful: store null-terminated token at *buf and return TRUE.
   * If no more tokens on line: set *buf = '\0' and return FALSE.
!  * If error: fill buf with truncated or misformatted token and return FALSE.
   */
  static bool
! next_token(char **lineptr, char *buf, int bufsz,
!            bool *initial_quote, bool *terminating_comma,
!            int elevel, char **err_msg)
  {
      int            c;
      char       *start_buf = buf;
*************** next_token(char **lineptr, char *buf, in
*** 197,210 ****
          if (buf >= end_buf)
          {
              *buf = '\0';
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                 errmsg("authentication file token too long, skipping: \"%s\"",
                        start_buf)));
              /* Discard remainder of line */
              while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
                  ;
!             break;
          }

          /* we do not pass back the comma in the token */
--- 239,253 ----
          if (buf >= end_buf)
          {
              *buf = '\0';
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                 errmsg("authentication file token too long, skipping: \"%s\"",
                        start_buf)));
+             *err_msg = "authentication file token too long";
              /* Discard remainder of line */
              while ((c = (*(*lineptr)++)) != '\0' && c != '\n')
                  ;
!             return false;
          }

          /* we do not pass back the comma in the token */
*************** next_token(char **lineptr, char *buf, in
*** 245,257 ****
      return (saw_quote || buf > start_buf);
  }

  static HbaToken *
! make_hba_token(char *token, bool quoted)
  {
      HbaToken   *hbatoken;
      int            toklen;

      toklen = strlen(token);
      hbatoken = (HbaToken *) palloc(sizeof(HbaToken) + toklen + 1);
      hbatoken->string = (char *) hbatoken + sizeof(HbaToken);
      hbatoken->quoted = quoted;
--- 288,304 ----
      return (saw_quote || buf > start_buf);
  }

+ /*
+  * Construct a palloc'd HbaToken struct, copying the given string.
+  */
  static HbaToken *
! make_hba_token(const char *token, bool quoted)
  {
      HbaToken   *hbatoken;
      int            toklen;

      toklen = strlen(token);
+     /* we copy string into same palloc block as the struct */
      hbatoken = (HbaToken *) palloc(sizeof(HbaToken) + toklen + 1);
      hbatoken->string = (char *) hbatoken + sizeof(HbaToken);
      hbatoken->quoted = quoted;
*************** copy_hba_token(HbaToken *in)
*** 275,285 ****
  /*
   * Tokenize one HBA field from a line, handling file inclusion and comma lists.
   *
!  * The result is a List of HbaToken structs for each individual token,
   * or NIL if we reached EOL.
   */
  static List *
! next_field_expand(const char *filename, char **lineptr)
  {
      char        buf[MAX_TOKEN];
      bool        trailing_comma;
--- 322,341 ----
  /*
   * Tokenize one HBA field from a line, handling file inclusion and comma lists.
   *
!  * filename: current file's pathname (needed to resolve relative pathnames)
!  * *lineptr: current line pointer, which will be advanced past field
!  *
!  * In event of an error, log a message at ereport level elevel, and also
!  * set *err_msg to a string describing the error.  Note that the result
!  * may be non-NIL anyway, so *err_msg must be tested to determine whether
!  * there was an error.
!  *
!  * The result is a List of HbaToken structs, one for each token in the field,
   * or NIL if we reached EOL.
   */
  static List *
! next_field_expand(const char *filename, char **lineptr,
!                   int elevel, char **err_msg)
  {
      char        buf[MAX_TOKEN];
      bool        trailing_comma;
*************** next_field_expand(const char *filename,
*** 288,302 ****

      do
      {
!         if (!next_token(lineptr, buf, sizeof(buf), &initial_quote, &trailing_comma))
              break;

          /* Is this referencing a file? */
          if (!initial_quote && buf[0] == '@' && buf[1] != '\0')
!             tokens = tokenize_inc_file(tokens, filename, buf + 1);
          else
              tokens = lappend(tokens, make_hba_token(buf, initial_quote));
!     } while (trailing_comma);

      return tokens;
  }
--- 344,361 ----

      do
      {
!         if (!next_token(lineptr, buf, sizeof(buf),
!                         &initial_quote, &trailing_comma,
!                         elevel, err_msg))
              break;

          /* Is this referencing a file? */
          if (!initial_quote && buf[0] == '@' && buf[1] != '\0')
!             tokens = tokenize_inc_file(tokens, filename, buf + 1,
!                                        elevel, err_msg);
          else
              tokens = lappend(tokens, make_hba_token(buf, initial_quote));
!     } while (trailing_comma && (*err_msg == NULL));

      return tokens;
  }
*************** next_field_expand(const char *filename,
*** 307,319 ****
   *
   * Opens and tokenises a file included from another HBA config file with @,
   * and returns all values found therein as a flat list of HbaTokens.  If a
!  * @-token is found, recursively expand it.  The given token list is used as
!  * initial contents of list (so foo,bar,@baz does what you expect).
   */
  static List *
  tokenize_inc_file(List *tokens,
                    const char *outer_filename,
!                   const char *inc_filename)
  {
      char       *inc_fullname;
      FILE       *inc_file;
--- 366,386 ----
   *
   * Opens and tokenises a file included from another HBA config file with @,
   * and returns all values found therein as a flat list of HbaTokens.  If a
!  * @-token is found, recursively expand it.  The newly read tokens are
!  * appended to "tokens" (so that foo,bar,@baz does what you expect).
!  * All new tokens are allocated in caller's memory context.
!  *
!  * In event of an error, log a message at ereport level elevel, and also
!  * set *err_msg to a string describing the error.  Note that the result
!  * may be non-NIL anyway, so *err_msg must be tested to determine whether
!  * there was an error.
   */
  static List *
  tokenize_inc_file(List *tokens,
                    const char *outer_filename,
!                   const char *inc_filename,
!                   int elevel,
!                   char **err_msg)
  {
      char       *inc_fullname;
      FILE       *inc_file;
*************** tokenize_inc_file(List *tokens,
*** 340,355 ****
      inc_file = AllocateFile(inc_fullname, "r");
      if (inc_file == NULL)
      {
!         ereport(LOG,
                  (errcode_for_file_access(),
                   errmsg("could not open secondary authentication file \"@%s\" as \"%s\": %m",
                          inc_filename, inc_fullname)));
          pfree(inc_fullname);
          return tokens;
      }

      /* There is possible recursion here if the file contains @ */
!     linecxt = tokenize_file(inc_fullname, inc_file, &inc_lines);

      FreeFile(inc_file);
      pfree(inc_fullname);
--- 407,426 ----
      inc_file = AllocateFile(inc_fullname, "r");
      if (inc_file == NULL)
      {
!         int            save_errno = errno;
!
!         ereport(elevel,
                  (errcode_for_file_access(),
                   errmsg("could not open secondary authentication file \"@%s\" as \"%s\": %m",
                          inc_filename, inc_fullname)));
+         *err_msg = psprintf("could not open secondary authentication file \"@%s\" as \"%s\": %s",
+                             inc_filename, inc_fullname, strerror(save_errno));
          pfree(inc_fullname);
          return tokens;
      }

      /* There is possible recursion here if the file contains @ */
!     linecxt = tokenize_file(inc_fullname, inc_file, &inc_lines, elevel);

      FreeFile(inc_file);
      pfree(inc_fullname);
*************** tokenize_inc_file(List *tokens,
*** 360,365 ****
--- 431,443 ----
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(inc_line);
          ListCell   *inc_field;

+         /* If any line has an error, propagate that up to caller */
+         if (tok_line->err_msg)
+         {
+             *err_msg = pstrdup(tok_line->err_msg);
+             break;
+         }
+
          foreach(inc_field, tok_line->fields)
          {
              List       *inc_tokens = lfirst(inc_field);
*************** tokenize_inc_file(List *tokens,
*** 383,395 ****
   *
   * The output is a list of TokenizedLine structs; see struct definition above.
   *
!  * filename must be the absolute path to the target file.
   *
   * Return value is a memory context which contains all memory allocated by
   * this function (it's a child of caller's context).
   */
  static MemoryContext
! tokenize_file(const char *filename, FILE *file, List **tok_lines)
  {
      int            line_number = 1;
      MemoryContext linecxt;
--- 461,480 ----
   *
   * The output is a list of TokenizedLine structs; see struct definition above.
   *
!  * filename: the absolute path to the target file
!  * file: the already-opened target file
!  * tok_lines: receives output list
!  * elevel: message logging level
!  *
!  * Errors are reported by logging messages at ereport level elevel and by
!  * adding TokenizedLine structs containing non-null err_msg fields to the
!  * output list.
   *
   * Return value is a memory context which contains all memory allocated by
   * this function (it's a child of caller's context).
   */
  static MemoryContext
! tokenize_file(const char *filename, FILE *file, List **tok_lines, int elevel)
  {
      int            line_number = 1;
      MemoryContext linecxt;
*************** tokenize_file(const char *filename, FILE
*** 407,422 ****
          char        rawline[MAX_LINE];
          char       *lineptr;
          List       *current_line = NIL;

          if (!fgets(rawline, sizeof(rawline), file))
!             break;
          if (strlen(rawline) == MAX_LINE - 1)
              /* Line too long! */
!             ereport(ERROR,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication file line too long"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_number, filename)));

          /* Strip trailing linebreak from rawline */
          lineptr = rawline + strlen(rawline) - 1;
--- 492,523 ----
          char        rawline[MAX_LINE];
          char       *lineptr;
          List       *current_line = NIL;
+         char       *err_msg = NULL;

          if (!fgets(rawline, sizeof(rawline), file))
!         {
!             int            save_errno = errno;
!
!             if (!ferror(file))
!                 break;            /* normal EOF */
!             /* I/O error! */
!             ereport(elevel,
!                     (errcode_for_file_access(),
!                      errmsg("could not read file \"%s\": %m", filename)));
!             err_msg = psprintf("could not read file \"%s\": %s",
!                                filename, strerror(save_errno));
!             rawline[0] = '\0';
!         }
          if (strlen(rawline) == MAX_LINE - 1)
+         {
              /* Line too long! */
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication file line too long"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_number, filename)));
+             err_msg = "authentication file line too long";
+         }

          /* Strip trailing linebreak from rawline */
          lineptr = rawline + strlen(rawline) - 1;
*************** tokenize_file(const char *filename, FILE
*** 425,442 ****

          /* Parse fields */
          lineptr = rawline;
!         while (*lineptr)
          {
              List       *current_field;

!             current_field = next_field_expand(filename, &lineptr);
              /* add field to line, unless we are at EOL or comment start */
              if (current_field != NIL)
                  current_line = lappend(current_line, current_field);
          }

          /* Reached EOL; emit line to TokenizedLine list unless it's boring */
!         if (current_line != NIL)
          {
              TokenizedLine *tok_line;

--- 526,544 ----

          /* Parse fields */
          lineptr = rawline;
!         while (*lineptr && err_msg == NULL)
          {
              List       *current_field;

!             current_field = next_field_expand(filename, &lineptr,
!                                               elevel, &err_msg);
              /* add field to line, unless we are at EOL or comment start */
              if (current_field != NIL)
                  current_line = lappend(current_line, current_field);
          }

          /* Reached EOL; emit line to TokenizedLine list unless it's boring */
!         if (current_line != NIL || err_msg != NULL)
          {
              TokenizedLine *tok_line;

*************** tokenize_file(const char *filename, FILE
*** 444,449 ****
--- 546,552 ----
              tok_line->fields = current_line;
              tok_line->line_num = line_number;
              tok_line->raw_line = pstrdup(rawline);
+             tok_line->err_msg = err_msg;
              *tok_lines = lappend(*tok_lines, tok_line);
          }

*************** check_same_host_or_net(SockAddr *raddr,
*** 746,751 ****
--- 849,858 ----

  /*
   * Macros used to check and report on invalid configuration options.
+  * On error: log a message at level elevel, set *err_msg, and exit the function.
+  * These macros are not as general-purpose as they look, because they know
+  * what the calling function's error-exit value is.
+  *
   * INVALID_AUTH_OPTION = reports when an option is specified for a method where it's
   *                         not supported.
   * REQUIRE_AUTH_OPTION = same as INVALID_AUTH_OPTION, except it also checks if the
*************** check_same_host_or_net(SockAddr *raddr,
*** 754,797 ****
   * MANDATORY_AUTH_ARG  = check if a required option is set for an authentication method,
   *                         reporting error if it's not.
   */
! #define INVALID_AUTH_OPTION(optname, validmethods) do {\
!     ereport(LOG, \
              (errcode(ERRCODE_CONFIG_FILE_ERROR), \
               /* translator: the second %s is a list of auth methods */ \
               errmsg("authentication option \"%s\" is only valid for authentication methods %s", \
                      optname, _(validmethods)), \
               errcontext("line %d of configuration file \"%s\"", \
                      line_num, HbaFileName))); \
      return false; \
! } while (0);

! #define REQUIRE_AUTH_OPTION(methodval, optname, validmethods) do {\
      if (hbaline->auth_method != methodval) \
          INVALID_AUTH_OPTION(optname, validmethods); \
! } while (0);

! #define MANDATORY_AUTH_ARG(argvar, argname, authname) do {\
!     if (argvar == NULL) {\
!         ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
                   errmsg("authentication method \"%s\" requires argument \"%s\" to be set", \
                          authname, argname), \
                   errcontext("line %d of configuration file \"%s\"", \
                          line_num, HbaFileName))); \
          return NULL; \
      } \
! } while (0);

  /*
   * IDENT_FIELD_ABSENT:
!  * Throw an error and exit the function if the given ident field ListCell is
   * not populated.
   *
   * IDENT_MULTI_VALUE:
!  * Throw an error and exit the function if the given ident token List has more
   * than one element.
   */
! #define IDENT_FIELD_ABSENT(field) do {\
      if (!field) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
--- 861,916 ----
   * MANDATORY_AUTH_ARG  = check if a required option is set for an authentication method,
   *                         reporting error if it's not.
   */
! #define INVALID_AUTH_OPTION(optname, validmethods) \
! do { \
!     ereport(elevel, \
              (errcode(ERRCODE_CONFIG_FILE_ERROR), \
               /* translator: the second %s is a list of auth methods */ \
               errmsg("authentication option \"%s\" is only valid for authentication methods %s", \
                      optname, _(validmethods)), \
               errcontext("line %d of configuration file \"%s\"", \
                      line_num, HbaFileName))); \
+     *err_msg = psprintf("authentication option \"%s\" is only valid for authentication methods %s", \
+                         optname, validmethods); \
      return false; \
! } while (0)

! #define REQUIRE_AUTH_OPTION(methodval, optname, validmethods) \
! do { \
      if (hbaline->auth_method != methodval) \
          INVALID_AUTH_OPTION(optname, validmethods); \
! } while (0)

! #define MANDATORY_AUTH_ARG(argvar, argname, authname) \
! do { \
!     if (argvar == NULL) { \
!         ereport(elevel, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
                   errmsg("authentication method \"%s\" requires argument \"%s\" to be set", \
                          authname, argname), \
                   errcontext("line %d of configuration file \"%s\"", \
                          line_num, HbaFileName))); \
+         *err_msg = psprintf("authentication method \"%s\" requires argument \"%s\" to be set", \
+                             authname, argname); \
          return NULL; \
      } \
! } while (0)

  /*
+  * Macros for handling pg_ident problems.
+  * Much as above, but currently the message level is hardwired as LOG
+  * and there is no provision for an err_msg string.
+  *
   * IDENT_FIELD_ABSENT:
!  * Log a message and exit the function if the given ident field ListCell is
   * not populated.
   *
   * IDENT_MULTI_VALUE:
!  * Log a message and exit the function if the given ident token List has more
   * than one element.
   */
! #define IDENT_FIELD_ABSENT(field) \
! do { \
      if (!field) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
*************** check_same_host_or_net(SockAddr *raddr,
*** 799,807 ****
                          IdentFileName, line_num))); \
          return NULL; \
      } \
! } while (0);

! #define IDENT_MULTI_VALUE(tokens) do {\
      if (tokens->length > 1) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
--- 918,927 ----
                          IdentFileName, line_num))); \
          return NULL; \
      } \
! } while (0)

! #define IDENT_MULTI_VALUE(tokens) \
! do { \
      if (tokens->length > 1) { \
          ereport(LOG, \
                  (errcode(ERRCODE_CONFIG_FILE_ERROR), \
*************** check_same_host_or_net(SockAddr *raddr,
*** 810,832 ****
                              line_num, IdentFileName))); \
          return NULL; \
      } \
! } while (0);


  /*
   * Parse one tokenised line from the hba config file and store the result in a
   * HbaLine structure.
   *
!  * Return NULL if parsing fails.
   *
   * Note: this function leaks memory when an error occurs.  Caller is expected
   * to have set a memory context that will be reset if this function returns
   * NULL.
   */
  static HbaLine *
! parse_hba_line(TokenizedLine *tok_line)
  {
      int            line_num = tok_line->line_num;
      char       *str;
      struct addrinfo *gai_result;
      struct addrinfo hints;
--- 930,955 ----
                              line_num, IdentFileName))); \
          return NULL; \
      } \
! } while (0)


  /*
   * Parse one tokenised line from the hba config file and store the result in a
   * HbaLine structure.
   *
!  * If parsing fails, log a message at ereport level elevel, store an error
!  * string in tok_line->err_msg, and return NULL.  (Some non-error conditions
!  * can also result in such messages.)
   *
   * Note: this function leaks memory when an error occurs.  Caller is expected
   * to have set a memory context that will be reset if this function returns
   * NULL.
   */
  static HbaLine *
! parse_hba_line(TokenizedLine *tok_line, int elevel)
  {
      int            line_num = tok_line->line_num;
+     char      **err_msg = &tok_line->err_msg;
      char       *str;
      struct addrinfo *gai_result;
      struct addrinfo hints;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 849,860 ****
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for connection type"),
                   errhint("Specify exactly one connection type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      token = linitial(tokens);
--- 972,984 ----
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for connection type"),
                   errhint("Specify exactly one connection type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "multiple values specified for connection type";
          return NULL;
      }
      token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 863,873 ****
  #ifdef HAVE_UNIX_SOCKETS
          parsedline->conntype = ctLocal;
  #else
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("local connections are not supported by this build"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
  #endif
      }
--- 987,998 ----
  #ifdef HAVE_UNIX_SOCKETS
          parsedline->conntype = ctLocal;
  #else
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("local connections are not supported by this build"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "local connections are not supported by this build";
          return NULL;
  #endif
      }
*************** parse_hba_line(TokenizedLine *tok_line)
*** 882,900 ****
              /* Log a warning if SSL support is not active */
  #ifdef USE_SSL
              if (!EnableSSL)
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                  errmsg("hostssl record cannot match because SSL is disabled"),
                           errhint("Set ssl = on in postgresql.conf."),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
  #else
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("hostssl record cannot match because SSL is not supported by this build"),
                errhint("Compile with --with-openssl to use SSL connections."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
  #endif
          }
          else if (token->string[4] == 'n')        /* "hostnossl" */
--- 1007,1029 ----
              /* Log a warning if SSL support is not active */
  #ifdef USE_SSL
              if (!EnableSSL)
!             {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                  errmsg("hostssl record cannot match because SSL is disabled"),
                           errhint("Set ssl = on in postgresql.conf."),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = "hostssl record cannot match because SSL is disabled";
+             }
  #else
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("hostssl record cannot match because SSL is not supported by this build"),
                errhint("Compile with --with-openssl to use SSL connections."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "hostssl record cannot match because SSL is not supported by this build";
  #endif
          }
          else if (token->string[4] == 'n')        /* "hostnossl" */
*************** parse_hba_line(TokenizedLine *tok_line)
*** 909,920 ****
      }                            /* record type */
      else
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid connection type \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1038,1050 ----
      }                            /* record type */
      else
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid connection type \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("invalid connection type \"%s\"", token->string);
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 922,932 ****
      field = lnext(field);
      if (!field)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before database specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      parsedline->databases = NIL;
--- 1052,1063 ----
      field = lnext(field);
      if (!field)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before database specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "end-of-line before database specification";
          return NULL;
      }
      parsedline->databases = NIL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 941,951 ****
      field = lnext(field);
      if (!field)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before role specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      parsedline->roles = NIL;
--- 1072,1083 ----
      field = lnext(field);
      if (!field)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before role specification"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "end-of-line before role specification";
          return NULL;
      }
      parsedline->roles = NIL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 962,983 ****
          field = lnext(field);
          if (!field)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("end-of-line before IP address specification"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
          tokens = lfirst(field);
          if (tokens->length > 1)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("multiple values specified for host address"),
                       errhint("Specify one address range per line."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
          token = linitial(tokens);
--- 1094,1117 ----
          field = lnext(field);
          if (!field)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("end-of-line before IP address specification"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "end-of-line before IP address specification";
              return NULL;
          }
          tokens = lfirst(field);
          if (tokens->length > 1)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("multiple values specified for host address"),
                       errhint("Specify one address range per line."),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "multiple values specified for host address";
              return NULL;
          }
          token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1027,1038 ****
                  parsedline->hostname = str;
              else
              {
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("invalid IP address \"%s\": %s",
                                  str, gai_strerror(ret)),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  if (gai_result)
                      pg_freeaddrinfo_all(hints.ai_family, gai_result);
                  return NULL;
--- 1161,1174 ----
                  parsedline->hostname = str;
              else
              {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("invalid IP address \"%s\": %s",
                                  str, gai_strerror(ret)),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = psprintf("invalid IP address \"%s\": %s",
+                                     str, gai_strerror(ret));
                  if (gai_result)
                      pg_freeaddrinfo_all(hints.ai_family, gai_result);
                  return NULL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1045,1068 ****
              {
                  if (parsedline->hostname)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("specifying both host name and CIDR mask is invalid: \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }

                  if (pg_sockaddr_cidr_mask(&parsedline->mask, cidr_slash + 1,
                                            parsedline->addr.ss_family) < 0)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid CIDR mask in address \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
                  pfree(str);
--- 1181,1208 ----
              {
                  if (parsedline->hostname)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("specifying both host name and CIDR mask is invalid: \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = psprintf("specifying both host name and CIDR mask is invalid: \"%s\"",
+                                         token->string);
                      return NULL;
                  }

                  if (pg_sockaddr_cidr_mask(&parsedline->mask, cidr_slash + 1,
                                            parsedline->addr.ss_family) < 0)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid CIDR mask in address \"%s\"",
                                      token->string),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = psprintf("invalid CIDR mask in address \"%s\"",
+                                         token->string);
                      return NULL;
                  }
                  pfree(str);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1074,1095 ****
                  field = lnext(field);
                  if (!field)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                            errmsg("end-of-line before netmask specification"),
                               errhint("Specify an address range in CIDR notation, or provide a separate netmask."),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
                  tokens = lfirst(field);
                  if (tokens->length > 1)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("multiple values specified for netmask"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
                  token = linitial(tokens);
--- 1214,1237 ----
                  field = lnext(field);
                  if (!field)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                            errmsg("end-of-line before netmask specification"),
                               errhint("Specify an address range in CIDR notation, or provide a separate netmask."),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = "end-of-line before netmask specification";
                      return NULL;
                  }
                  tokens = lfirst(field);
                  if (tokens->length > 1)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("multiple values specified for netmask"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = "multiple values specified for netmask";
                      return NULL;
                  }
                  token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1098,1109 ****
                                           &hints, &gai_result);
                  if (ret || !gai_result)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid IP mask \"%s\": %s",
                                      token->string, gai_strerror(ret)),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      if (gai_result)
                          pg_freeaddrinfo_all(hints.ai_family, gai_result);
                      return NULL;
--- 1240,1253 ----
                                           &hints, &gai_result);
                  if (ret || !gai_result)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("invalid IP mask \"%s\": %s",
                                      token->string, gai_strerror(ret)),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = psprintf("invalid IP mask \"%s\": %s",
+                                         token->string, gai_strerror(ret));
                      if (gai_result)
                          pg_freeaddrinfo_all(hints.ai_family, gai_result);
                      return NULL;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1115,1125 ****

                  if (parsedline->addr.ss_family != parsedline->mask.ss_family)
                  {
!                     ereport(LOG,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("IP address and mask do not match"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
                      return NULL;
                  }
              }
--- 1259,1270 ----

                  if (parsedline->addr.ss_family != parsedline->mask.ss_family)
                  {
!                     ereport(elevel,
                              (errcode(ERRCODE_CONFIG_FILE_ERROR),
                               errmsg("IP address and mask do not match"),
                             errcontext("line %d of configuration file \"%s\"",
                                        line_num, HbaFileName)));
+                     *err_msg = "IP address and mask do not match";
                      return NULL;
                  }
              }
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1130,1151 ****
      field = lnext(field);
      if (!field)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before authentication method"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for authentication type"),
                   errhint("Specify exactly one authentication type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }
      token = linitial(tokens);
--- 1275,1298 ----
      field = lnext(field);
      if (!field)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("end-of-line before authentication method"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "end-of-line before authentication method";
          return NULL;
      }
      tokens = lfirst(field);
      if (tokens->length > 1)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("multiple values specified for authentication type"),
                   errhint("Specify exactly one authentication type per line."),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "multiple values specified for authentication type";
          return NULL;
      }
      token = linitial(tokens);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1177,1187 ****
      {
          if (Db_user_namespace)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
          parsedline->auth_method = uaMD5;
--- 1324,1335 ----
      {
          if (Db_user_namespace)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled";
              return NULL;
          }
          parsedline->auth_method = uaMD5;
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1214,1236 ****
          parsedline->auth_method = uaRADIUS;
      else
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

      if (unsupauth)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\": not supported by this build",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1362,1388 ----
          parsedline->auth_method = uaRADIUS;
      else
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\"",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("invalid authentication method \"%s\"",
+                             token->string);
          return NULL;
      }

      if (unsupauth)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("invalid authentication method \"%s\": not supported by this build",
                          token->string),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("invalid authentication method \"%s\": not supported by this build",
+                             token->string);
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 1246,1267 ****
      if (parsedline->conntype == ctLocal &&
          parsedline->auth_method == uaGSS)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
             errmsg("gssapi authentication is not supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

      if (parsedline->conntype != ctLocal &&
          parsedline->auth_method == uaPeer)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("peer authentication is only supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1398,1421 ----
      if (parsedline->conntype == ctLocal &&
          parsedline->auth_method == uaGSS)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
             errmsg("gssapi authentication is not supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "gssapi authentication is not supported on local sockets";
          return NULL;
      }

      if (parsedline->conntype != ctLocal &&
          parsedline->auth_method == uaPeer)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("peer authentication is only supported on local sockets"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "peer authentication is only supported on local sockets";
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 1274,1284 ****
      if (parsedline->conntype != ctHostSSL &&
          parsedline->auth_method == uaCert)
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("cert authentication is only supported on hostssl connections"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return NULL;
      }

--- 1428,1439 ----
      if (parsedline->conntype != ctHostSSL &&
          parsedline->auth_method == uaCert)
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("cert authentication is only supported on hostssl connections"),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = "cert authentication is only supported on hostssl connections";
          return NULL;
      }

*************** parse_hba_line(TokenizedLine *tok_line)
*** 1323,1338 ****
                  /*
                   * Got something that's not a name=value pair.
                   */
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("authentication option not in name=value format: %s", token->string),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  return NULL;
              }

              *val++ = '\0';        /* str now holds "name", val holds "value" */
!             if (!parse_hba_auth_opt(str, val, parsedline, line_num))
                  /* parse_hba_auth_opt already logged the error message */
                  return NULL;
              pfree(str);
--- 1478,1495 ----
                  /*
                   * Got something that's not a name=value pair.
                   */
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("authentication option not in name=value format: %s", token->string),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = psprintf("authentication option not in name=value format: %s",
+                                     token->string);
                  return NULL;
              }

              *val++ = '\0';        /* str now holds "name", val holds "value" */
!             if (!parse_hba_auth_opt(str, val, parsedline, elevel, err_msg))
                  /* parse_hba_auth_opt already logged the error message */
                  return NULL;
              pfree(str);
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1360,1380 ****
                  parsedline->ldapbindpasswd ||
                  parsedline->ldapsearchattribute)
              {
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl
togetherwith ldapprefix"), 
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  return NULL;
              }
          }
          else if (!parsedline->ldapbasedn)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or
\"ldapsuffix\"to be set"), 
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return NULL;
          }
      }
--- 1517,1539 ----
                  parsedline->ldapbindpasswd ||
                  parsedline->ldapsearchattribute)
              {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl
togetherwith ldapprefix"), 
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = "cannot use ldapbasedn, ldapbinddn, ldapbindpasswd, ldapsearchattribute, or ldapurl
togetherwith ldapprefix"; 
                  return NULL;
              }
          }
          else if (!parsedline->ldapbasedn)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or
\"ldapsuffix\"to be set"), 
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "authentication method \"ldap\" requires argument \"ldapbasedn\", \"ldapprefix\", or
\"ldapsuffix\"to be set"; 
              return NULL;
          }
      }
*************** parse_hba_line(TokenizedLine *tok_line)
*** 1399,1409 ****
  /*
   * Parse one name-value pair as an authentication option into the given
   * HbaLine.  Return true if we successfully parse the option, false if we
!  * encounter an error.
   */
  static bool
! parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline, int line_num)
  {
  #ifdef USE_LDAP
      hbaline->ldapscope = LDAP_SCOPE_SUBTREE;
  #endif
--- 1558,1572 ----
  /*
   * Parse one name-value pair as an authentication option into the given
   * HbaLine.  Return true if we successfully parse the option, false if we
!  * encounter an error.  In the event of an error, also log a message at
!  * ereport level elevel, and store a message string into *err_msg.
   */
  static bool
! parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
!                    int elevel, char **err_msg)
  {
+     int            line_num = hbaline->linenumber;
+
  #ifdef USE_LDAP
      hbaline->ldapscope = LDAP_SCOPE_SUBTREE;
  #endif
*************** parse_hba_auth_opt(char *name, char *val
*** 1422,1432 ****
      {
          if (hbaline->conntype != ctHostSSL)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("clientcert can only be configured for \"hostssl\" rows"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return false;
          }
          if (strcmp(val, "1") == 0)
--- 1585,1596 ----
      {
          if (hbaline->conntype != ctHostSSL)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("clientcert can only be configured for \"hostssl\" rows"),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = "clientcert can only be configured for \"hostssl\" rows";
              return false;
          }
          if (strcmp(val, "1") == 0)
*************** parse_hba_auth_opt(char *name, char *val
*** 1437,1447 ****
          {
              if (hbaline->auth_method == uaCert)
              {
!                 ereport(LOG,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("clientcert can not be set to 0 when using \"cert\" authentication"),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
                  return false;
              }
              hbaline->clientcert = false;
--- 1601,1612 ----
          {
              if (hbaline->auth_method == uaCert)
              {
!                 ereport(elevel,
                          (errcode(ERRCODE_CONFIG_FILE_ERROR),
                           errmsg("clientcert can not be set to 0 when using \"cert\" authentication"),
                           errcontext("line %d of configuration file \"%s\"",
                                      line_num, HbaFileName)));
+                 *err_msg = "clientcert can not be set to 0 when using \"cert\" authentication";
                  return false;
              }
              hbaline->clientcert = false;
*************** parse_hba_auth_opt(char *name, char *val
*** 1473,1489 ****
          rc = ldap_url_parse(val, &urldata);
          if (rc != LDAP_SUCCESS)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not parse LDAP URL \"%s\": %s", val, ldap_err2string(rc))));
              return false;
          }

          if (strcmp(urldata->lud_scheme, "ldap") != 0)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("unsupported LDAP URL scheme: %s", urldata->lud_scheme)));
              ldap_free_urldesc(urldata);
              return false;
          }
--- 1638,1658 ----
          rc = ldap_url_parse(val, &urldata);
          if (rc != LDAP_SUCCESS)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not parse LDAP URL \"%s\": %s", val, ldap_err2string(rc))));
+             *err_msg = psprintf("could not parse LDAP URL \"%s\": %s",
+                                 val, ldap_err2string(rc));
              return false;
          }

          if (strcmp(urldata->lud_scheme, "ldap") != 0)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
              errmsg("unsupported LDAP URL scheme: %s", urldata->lud_scheme)));
+             *err_msg = psprintf("unsupported LDAP URL scheme: %s",
+                                 urldata->lud_scheme);
              ldap_free_urldesc(urldata);
              return false;
          }
*************** parse_hba_auth_opt(char *name, char *val
*** 1497,1513 ****
          hbaline->ldapscope = urldata->lud_scope;
          if (urldata->lud_filter)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("filters not supported in LDAP URLs")));
              ldap_free_urldesc(urldata);
              return false;
          }
          ldap_free_urldesc(urldata);
  #else                            /* not OpenLDAP */
!         ereport(LOG,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                   errmsg("LDAP URLs not supported on this platform")));
  #endif   /* not OpenLDAP */
      }
      else if (strcmp(name, "ldaptls") == 0)
--- 1666,1684 ----
          hbaline->ldapscope = urldata->lud_scope;
          if (urldata->lud_filter)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("filters not supported in LDAP URLs")));
+             *err_msg = "filters not supported in LDAP URLs";
              ldap_free_urldesc(urldata);
              return false;
          }
          ldap_free_urldesc(urldata);
  #else                            /* not OpenLDAP */
!         ereport(elevel,
                  (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                   errmsg("LDAP URLs not supported on this platform")));
+         *err_msg = "LDAP URLs not supported on this platform";
  #endif   /* not OpenLDAP */
      }
      else if (strcmp(name, "ldaptls") == 0)
*************** parse_hba_auth_opt(char *name, char *val
*** 1529,1539 ****
          hbaline->ldapport = atoi(val);
          if (hbaline->ldapport == 0)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid LDAP port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return false;
          }
      }
--- 1700,1711 ----
          hbaline->ldapport = atoi(val);
          if (hbaline->ldapport == 0)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid LDAP port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = psprintf("invalid LDAP port number: \"%s\"", val);
              return false;
          }
      }
*************** parse_hba_auth_opt(char *name, char *val
*** 1617,1628 ****
          ret = pg_getaddrinfo_all(val, NULL, &hints, &gai_result);
          if (ret || !gai_result)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not translate RADIUS server name \"%s\" to address: %s",
                              val, gai_strerror(ret)),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              if (gai_result)
                  pg_freeaddrinfo_all(hints.ai_family, gai_result);
              return false;
--- 1789,1802 ----
          ret = pg_getaddrinfo_all(val, NULL, &hints, &gai_result);
          if (ret || !gai_result)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("could not translate RADIUS server name \"%s\" to address: %s",
                              val, gai_strerror(ret)),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = psprintf("could not translate RADIUS server name \"%s\" to address: %s",
+                                 val, gai_strerror(ret));
              if (gai_result)
                  pg_freeaddrinfo_all(hints.ai_family, gai_result);
              return false;
*************** parse_hba_auth_opt(char *name, char *val
*** 1636,1646 ****
          hbaline->radiusport = atoi(val);
          if (hbaline->radiusport == 0)
          {
!             ereport(LOG,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid RADIUS port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
              return false;
          }
      }
--- 1810,1821 ----
          hbaline->radiusport = atoi(val);
          if (hbaline->radiusport == 0)
          {
!             ereport(elevel,
                      (errcode(ERRCODE_CONFIG_FILE_ERROR),
                       errmsg("invalid RADIUS port number: \"%s\"", val),
                       errcontext("line %d of configuration file \"%s\"",
                                  line_num, HbaFileName)));
+             *err_msg = psprintf("invalid RADIUS port number: \"%s\"", val);
              return false;
          }
      }
*************** parse_hba_auth_opt(char *name, char *val
*** 1656,1667 ****
      }
      else
      {
!         ereport(LOG,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("unrecognized authentication option name: \"%s\"",
                          name),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
          return false;
      }
      return true;
--- 1831,1844 ----
      }
      else
      {
!         ereport(elevel,
                  (errcode(ERRCODE_CONFIG_FILE_ERROR),
                   errmsg("unrecognized authentication option name: \"%s\"",
                          name),
                   errcontext("line %d of configuration file \"%s\"",
                              line_num, HbaFileName)));
+         *err_msg = psprintf("unrecognized authentication option name: \"%s\"",
+                             name);
          return false;
      }
      return true;
*************** load_hba(void)
*** 1794,1800 ****
          return false;
      }

!     linecxt = tokenize_file(HbaFileName, file, &hba_lines);
      FreeFile(file);

      /* Now parse all the lines */
--- 1971,1977 ----
          return false;
      }

!     linecxt = tokenize_file(HbaFileName, file, &hba_lines, LOG);
      FreeFile(file);

      /* Now parse all the lines */
*************** load_hba(void)
*** 1808,1828 ****
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line);
          HbaLine    *newline;

!         if ((newline = parse_hba_line(tok_line)) == NULL)
          {
!             /*
!              * Parse error in the file, so indicate there's a problem.  NB: a
!              * problem in a line will free the memory for all previous lines
!              * as well!
!              */
!             MemoryContextReset(hbacxt);
!             new_parsed_lines = NIL;
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first row. Error has already been reported in the
!              * parsing function, so no need to log it here.
               */
              continue;
          }
--- 1985,2006 ----
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line);
          HbaLine    *newline;

!         /* don't parse lines that already have errors */
!         if (tok_line->err_msg != NULL)
          {
!             ok = false;
!             continue;
!         }
!
!         if ((newline = parse_hba_line(tok_line, LOG)) == NULL)
!         {
!             /* Parse error; remember there's trouble */
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first line.  Error has already been logged, no
!              * need for more chatter here.
               */
              continue;
          }
*************** load_hba(void)
*** 1865,1874 ****
  }

  /*
   * Parse one tokenised line from the ident config file and store the result in
   * an IdentLine structure.
   *
!  * Return NULL if parsing fails.
   *
   * If ident_user is a regular expression (ie. begins with a slash), it is
   * compiled and stored in IdentLine structure.
--- 2043,2454 ----
  }

  /*
+  * This macro specifies the maximum number of authentication options
+  * that are possible with any given authentication method that is supported.
+  * Currently LDAP supports 10, so the macro value is well above the most any
+  * method needs.
+  */
+ #define MAX_HBA_OPTIONS 12
+
+ /*
+  * Create a text array listing the options specified in the HBA line.
+  * Return NULL if no options are specified.
+  */
+ static ArrayType *
+ gethba_options(HbaLine *hba)
+ {
+     int            noptions;
+     Datum        options[MAX_HBA_OPTIONS];
+
+     noptions = 0;
+
+     if (hba->auth_method == uaGSS || hba->auth_method == uaSSPI)
+     {
+         if (hba->include_realm)
+             options[noptions++] =
+                 CStringGetTextDatum("include_realm=true");
+
+         if (hba->krb_realm)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("krb_realm=%s", hba->krb_realm));
+     }
+
+     if (hba->usermap)
+         options[noptions++] =
+             CStringGetTextDatum(psprintf("map=%s", hba->usermap));
+
+     if (hba->clientcert)
+         options[noptions++] =
+             CStringGetTextDatum("clientcert=true");
+
+     if (hba->pamservice)
+         options[noptions++] =
+             CStringGetTextDatum(psprintf("pamservice=%s", hba->pamservice));
+
+     if (hba->auth_method == uaLDAP)
+     {
+         if (hba->ldapserver)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapserver=%s", hba->ldapserver));
+
+         if (hba->ldapport)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapport=%d", hba->ldapport));
+
+         if (hba->ldaptls)
+             options[noptions++] =
+                 CStringGetTextDatum("ldaptls=true");
+
+         if (hba->ldapprefix)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapprefix=%s", hba->ldapprefix));
+
+         if (hba->ldapsuffix)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapsuffix=%s", hba->ldapsuffix));
+
+         if (hba->ldapbasedn)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapbasedn=%s", hba->ldapbasedn));
+
+         if (hba->ldapbinddn)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapbinddn=%s", hba->ldapbinddn));
+
+         if (hba->ldapbindpasswd)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapbindpasswd=%s",
+                                              hba->ldapbindpasswd));
+
+         if (hba->ldapsearchattribute)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapsearchattribute=%s",
+                                              hba->ldapsearchattribute));
+
+         if (hba->ldapscope)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("ldapscope=%d", hba->ldapscope));
+     }
+
+     if (hba->auth_method == uaRADIUS)
+     {
+         if (hba->radiusserver)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiusserver=%s", hba->radiusserver));
+
+         if (hba->radiussecret)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiussecret=%s", hba->radiussecret));
+
+         if (hba->radiusidentifier)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiusidentifier=%s", hba->radiusidentifier));
+
+         if (hba->radiusport)
+             options[noptions++] =
+                 CStringGetTextDatum(psprintf("radiusport=%d", hba->radiusport));
+     }
+
+     Assert(noptions <= MAX_HBA_OPTIONS);
+
+     if (noptions > 0)
+         return construct_array(options, noptions, TEXTOID, -1, false, 'i');
+     else
+         return NULL;
+ }
+
+ /* Number of columns in pg_hba_file_rules view */
+ #define NUM_PG_HBA_FILE_RULES_ATTS     9
+
+ /*
+  * fill_hba_line: build one row of pg_hba_file_rules view, add it to tuplestore
+  *
+  * tuple_store: where to store data
+  * tupdesc: tuple descriptor for the view
+  * lineno: pg_hba.conf line number (must always be valid)
+  * hba: parsed line data (can be NULL, in which case err_msg should be set)
+  * err_msg: error message (NULL if none)
+  *
+  * Note: leaks memory, but we don't care since this is run in a short-lived
+  * memory context.
+  */
+ static void
+ fill_hba_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
+               int lineno, HbaLine *hba, const char *err_msg)
+ {
+     Datum        values[NUM_PG_HBA_FILE_RULES_ATTS];
+     bool        nulls[NUM_PG_HBA_FILE_RULES_ATTS];
+     char        buffer[NI_MAXHOST];
+     HeapTuple    tuple;
+     int            index;
+     ListCell   *lc;
+     const char *typestr;
+     const char *addrstr;
+     const char *maskstr;
+     ArrayType  *options;
+
+     Assert(tupdesc->natts == NUM_PG_HBA_FILE_RULES_ATTS);
+
+     memset(values, 0, sizeof(values));
+     memset(nulls, 0, sizeof(nulls));
+     index = 0;
+
+     /* line_number */
+     values[index++] = Int32GetDatum(lineno);
+
+     if (hba != NULL)
+     {
+         /* type */
+         /* Avoid a default: case so compiler will warn about missing cases */
+         typestr = NULL;
+         switch (hba->conntype)
+         {
+             case ctLocal:
+                 typestr = "local";
+                 break;
+             case ctHost:
+                 typestr = "host";
+                 break;
+             case ctHostSSL:
+                 typestr = "hostssl";
+                 break;
+             case ctHostNoSSL:
+                 typestr = "hostnossl";
+                 break;
+         }
+         if (typestr)
+             values[index++] = CStringGetTextDatum(typestr);
+         else
+             nulls[index++] = true;
+
+         /* database */
+         if (hba->databases)
+         {
+             /*
+              * Flatten HbaToken list to string list.  It might seem that we
+              * should re-quote any quoted tokens, but that has been rejected
+              * on the grounds that it makes it harder to compare the array
+              * elements to other system catalogs.  That makes entries like
+              * "all" or "samerole" formally ambiguous ... but users who name
+              * databases/roles that way are inflicting their own pain.
+              */
+             List       *names = NIL;
+
+             foreach(lc, hba->databases)
+             {
+                 HbaToken   *tok = lfirst(lc);
+
+                 names = lappend(names, tok->string);
+             }
+             values[index++] = PointerGetDatum(strlist_to_textarray(names));
+         }
+         else
+             nulls[index++] = true;
+
+         /* user */
+         if (hba->roles)
+         {
+             /* Flatten HbaToken list to string list; see comment above */
+             List       *roles = NIL;
+
+             foreach(lc, hba->roles)
+             {
+                 HbaToken   *tok = lfirst(lc);
+
+                 roles = lappend(roles, tok->string);
+             }
+             values[index++] = PointerGetDatum(strlist_to_textarray(roles));
+         }
+         else
+             nulls[index++] = true;
+
+         /* address and netmask */
+         /* Avoid a default: case so compiler will warn about missing cases */
+         addrstr = maskstr = NULL;
+         switch (hba->ip_cmp_method)
+         {
+             case ipCmpMask:
+                 if (hba->hostname)
+                 {
+                     addrstr = hba->hostname;
+                 }
+                 else
+                 {
+                     if (pg_getnameinfo_all(&hba->addr, sizeof(hba->addr),
+                                            buffer, sizeof(buffer),
+                                            NULL, 0,
+                                            NI_NUMERICHOST) == 0)
+                     {
+                         clean_ipv6_addr(hba->addr.ss_family, buffer);
+                         addrstr = pstrdup(buffer);
+                     }
+                     if (pg_getnameinfo_all(&hba->mask, sizeof(hba->mask),
+                                            buffer, sizeof(buffer),
+                                            NULL, 0,
+                                            NI_NUMERICHOST) == 0)
+                     {
+                         clean_ipv6_addr(hba->mask.ss_family, buffer);
+                         maskstr = pstrdup(buffer);
+                     }
+                 }
+                 break;
+             case ipCmpAll:
+                 addrstr = "all";
+                 break;
+             case ipCmpSameHost:
+                 addrstr = "samehost";
+                 break;
+             case ipCmpSameNet:
+                 addrstr = "samenet";
+                 break;
+         }
+         if (addrstr)
+             values[index++] = CStringGetTextDatum(addrstr);
+         else
+             nulls[index++] = true;
+         if (maskstr)
+             values[index++] = CStringGetTextDatum(maskstr);
+         else
+             nulls[index++] = true;
+
+         /* auth_method */
+         values[index++] = CStringGetTextDatum(UserAuthName[hba->auth_method]);
+
+         /* options */
+         options = gethba_options(hba);
+         if (options)
+             values[index++] = PointerGetDatum(options);
+         else
+             nulls[index++] = true;
+     }
+     else
+     {
+         /* no parsing result, so set relevant fields to nulls */
+         memset(&nulls[1], true, (NUM_PG_HBA_FILE_RULES_ATTS - 2) * sizeof(bool));
+     }
+
+     /* error */
+     if (err_msg)
+         values[NUM_PG_HBA_FILE_RULES_ATTS - 1] = CStringGetTextDatum(err_msg);
+     else
+         nulls[NUM_PG_HBA_FILE_RULES_ATTS - 1] = true;
+
+     tuple = heap_form_tuple(tupdesc, values, nulls);
+     tuplestore_puttuple(tuple_store, tuple);
+ }
+
+ /*
+  * Read the pg_hba.conf file and fill the tuplestore with view records.
+  */
+ static void
+ fill_hba_view(Tuplestorestate *tuple_store, TupleDesc tupdesc)
+ {
+     FILE       *file;
+     List       *hba_lines = NIL;
+     ListCell   *line;
+     MemoryContext linecxt;
+     MemoryContext hbacxt;
+     MemoryContext oldcxt;
+
+     /*
+      * In the unlikely event that we can't open pg_hba.conf, we throw an
+      * error, rather than trying to report it via some sort of view entry.
+      * (Most other error conditions should result in a message in a view
+      * entry.)
+      */
+     file = AllocateFile(HbaFileName, "r");
+     if (file == NULL)
+         ereport(ERROR,
+                 (errcode_for_file_access(),
+                  errmsg("could not open configuration file \"%s\": %m",
+                         HbaFileName)));
+
+     linecxt = tokenize_file(HbaFileName, file, &hba_lines, DEBUG3);
+     FreeFile(file);
+
+     /* Now parse all the lines */
+     hbacxt = AllocSetContextCreate(CurrentMemoryContext,
+                                    "hba parser context",
+                                    ALLOCSET_SMALL_SIZES);
+     oldcxt = MemoryContextSwitchTo(hbacxt);
+     foreach(line, hba_lines)
+     {
+         TokenizedLine *tok_line = (TokenizedLine *) lfirst(line);
+         HbaLine    *hbaline = NULL;
+
+         /* don't parse lines that already have errors */
+         if (tok_line->err_msg == NULL)
+             hbaline = parse_hba_line(tok_line, DEBUG3);
+
+         fill_hba_line(tuple_store, tupdesc, tok_line->line_num,
+                       hbaline, tok_line->err_msg);
+     }
+
+     /* Free tokenizer memory */
+     MemoryContextDelete(linecxt);
+     /* Free parse_hba_line memory */
+     MemoryContextSwitchTo(oldcxt);
+     MemoryContextDelete(hbacxt);
+ }
+
+ /*
+  * SQL-accessible SRF to return all the entries in the pg_hba.conf file.
+  */
+ Datum
+ pg_hba_file_rules(PG_FUNCTION_ARGS)
+ {
+     Tuplestorestate *tuple_store;
+     TupleDesc    tupdesc;
+     MemoryContext old_cxt;
+     ReturnSetInfo *rsi;
+
+     /*
+      * We must use the Materialize mode to be safe against HBA file changes
+      * while the cursor is open. It's also more efficient than having to look
+      * up our current position in the parsed list every time.
+      */
+     rsi = (ReturnSetInfo *) fcinfo->resultinfo;
+
+     /* Check to see if caller supports us returning a tuplestore */
+     if (rsi == NULL || !IsA(rsi, ReturnSetInfo))
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("set-valued function called in context that cannot accept a set")));
+     if (!(rsi->allowedModes & SFRM_Materialize))
+         ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("materialize mode required, but it is not " \
+                         "allowed in this context")));
+
+     rsi->returnMode = SFRM_Materialize;
+
+     /* Build a tuple descriptor for our result type */
+     if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
+         elog(ERROR, "return type must be a row type");
+
+     /* Build tuplestore to hold the result rows */
+     old_cxt = MemoryContextSwitchTo(rsi->econtext->ecxt_per_query_memory);
+
+     tuple_store =
+         tuplestore_begin_heap(rsi->allowedModes & SFRM_Materialize_Random,
+                               false, work_mem);
+     rsi->setDesc = tupdesc;
+     rsi->setResult = tuple_store;
+
+     MemoryContextSwitchTo(old_cxt);
+
+     /* Fill the tuplestore */
+     fill_hba_view(tuple_store, tupdesc);
+
+     PG_RETURN_NULL();
+ }
+
+
+ /*
   * Parse one tokenised line from the ident config file and store the result in
   * an IdentLine structure.
   *
!  * If parsing fails, log a message and return NULL.
   *
   * If ident_user is a regular expression (ie. begins with a slash), it is
   * compiled and stored in IdentLine structure.
*************** load_ident(void)
*** 2170,2176 ****
          return false;
      }

!     linecxt = tokenize_file(IdentFileName, file, &ident_lines);
      FreeFile(file);

      /* Now parse all the lines */
--- 2750,2756 ----
          return false;
      }

!     linecxt = tokenize_file(IdentFileName, file, &ident_lines, LOG);
      FreeFile(file);

      /* Now parse all the lines */
*************** load_ident(void)
*** 2183,2208 ****
      {
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line_cell);

          if ((newline = parse_ident_line(tok_line)) == NULL)
          {
!             /*
!              * Parse error in the file, so indicate there's a problem.  Free
!              * all the memory and regular expressions of lines parsed so far.
!              */
!             foreach(parsed_line_cell, new_parsed_lines)
!             {
!                 newline = (IdentLine *) lfirst(parsed_line_cell);
!                 if (newline->ident_user[0] == '/')
!                     pg_regfree(&newline->re);
!             }
!             MemoryContextReset(ident_context);
!             new_parsed_lines = NIL;
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first row. Error has already been reported in the
!              * parsing function, so no need to log it here.
               */
              continue;
          }
--- 2763,2784 ----
      {
          TokenizedLine *tok_line = (TokenizedLine *) lfirst(line_cell);

+         /* don't parse lines that already have errors */
+         if (tok_line->err_msg != NULL)
+         {
+             ok = false;
+             continue;
+         }
+
          if ((newline = parse_ident_line(tok_line)) == NULL)
          {
!             /* Parse error; remember there's trouble */
              ok = false;

              /*
               * Keep parsing the rest of the file so we can report errors on
!              * more than the first line.  Error has already been logged, no
!              * need for more chatter here.
               */
              continue;
          }
*************** load_ident(void)
*** 2216,2222 ****

      if (!ok)
      {
!         /* File contained one or more errors, so bail out */
          foreach(parsed_line_cell, new_parsed_lines)
          {
              newline = (IdentLine *) lfirst(parsed_line_cell);
--- 2792,2802 ----

      if (!ok)
      {
!         /*
!          * File contained one or more errors, so bail out, first being careful
!          * to clean up whatever we allocated.  Most stuff will go away via
!          * MemoryContextDelete, but we have to clean up regexes explicitly.
!          */
          foreach(parsed_line_cell, new_parsed_lines)
          {
              newline = (IdentLine *) lfirst(parsed_line_cell);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 31c828a..05652e8 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DATA(insert OID = 2084 (  pg_show_all_se
*** 3076,3081 ****
--- 3076,3083 ----
  DESCR("SHOW ALL as a function");
  DATA(insert OID = 3329 (  pg_show_all_file_settings PGNSP PGUID 12 1 1000 0 0 f f f f t t v s 0 0 2249 ""
"{25,23,23,25,25,16,25}""{o,o,o,o,o,o,o}" "{sourcefile,sourceline,seqno,name,setting,applied,error}" _null_ _null_
show_all_file_settings_null_ _null_ _null_ )); 
  DESCR("show config file settings");
+ DATA(insert OID = 3401 (  pg_hba_file_rules PGNSP PGUID 12 1 1000 0 0 f f f f t t v s 0 0 2249 ""
"{23,25,1009,1009,25,25,25,1009,25}""{o,o,o,o,o,o,o,o,o}"
"{line_number,type,database,user_name,address,netmask,auth_method,options,error}"_null_ _null_ pg_hba_file_rules _null_
_null__null_ )); 
+ DESCR("show pg_hba.conf rules");
  DATA(insert OID = 1371 (  pg_lock_status   PGNSP PGUID 12 1 1000 0 0 f f f f t t v s 0 0 2249 ""
"{25,26,26,23,21,25,28,26,26,21,25,23,25,16,16}""{o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}"
"{locktype,database,relation,page,tuple,virtualxid,transactionid,classid,objid,objsubid,virtualtransaction,pid,mode,granted,fastpath}"
_null__null_ pg_lock_status _null_ _null_ _null_ )); 
  DESCR("view system lock information");
  DATA(insert OID = 2561 (  pg_blocking_pids PGNSP PGUID 12 1 0 0 0 f f f f t f v s 1 0 1007 "23" _null_ _null_ _null_
_null__null_ pg_blocking_pids _null_ _null_ _null_ )); 
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index dc7d257..893767f 100644
*** a/src/include/libpq/hba.h
--- b/src/include/libpq/hba.h
***************
*** 16,25 ****
  #include "regex/regex.h"


  typedef enum UserAuth
  {
      uaReject,
!     uaImplicitReject,
      uaTrust,
      uaIdent,
      uaPassword,
--- 16,31 ----
  #include "regex/regex.h"


+ /*
+  * The following enum represents the authentication methods that
+  * are supported by PostgreSQL.
+  *
+  * Note: keep this in sync with the UserAuthName array in hba.c.
+  */
  typedef enum UserAuth
  {
      uaReject,
!     uaImplicitReject,            /* Not a user-visible option */
      uaTrust,
      uaIdent,
      uaPassword,
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 60abcad..de5ae00 100644
*** a/src/test/regress/expected/rules.out
--- b/src/test/regress/expected/rules.out
*************** pg_group| SELECT pg_authid.rolname AS gr
*** 1338,1343 ****
--- 1338,1353 ----
            WHERE (pg_auth_members.roleid = pg_authid.oid)) AS grolist
     FROM pg_authid
    WHERE (NOT pg_authid.rolcanlogin);
+ pg_hba_file_rules| SELECT a.line_number,
+     a.type,
+     a.database,
+     a.user_name,
+     a.address,
+     a.netmask,
+     a.auth_method,
+     a.options,
+     a.error
+    FROM pg_hba_file_rules() a(line_number, type, database, user_name, address, netmask, auth_method, options, error);
  pg_indexes| SELECT n.nspname AS schemaname,
      c.relname AS tablename,
      i.relname AS indexname,

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

On Mon, Jan 30, 2017 at 11:20 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> On Sun, Jan 29, 2017 at 9:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> tgl wrote:
>> > I spent awhile hacking on this, and made a lot of things better, but
>> > I'm still very unhappy about the state of the comments.
>>
>> I made another pass over this, working on the comments and the docs,
>> and changing the view name to "pg_hba_file_rules".  I think this version
>> is committable if people are satisfied with that name.

(catching up with this thread as a lot has happened.)

> Thanks for working on the patch. I am fine with the "pg_hba_file_rules"
> name. I have to improve in writing better comments after checking the
> attached patch. I will improve the comments in further patch submissions
> to community.

No objections here.

+/*
+ * The following character array represents the names of the authentication
+ * methods that are supported by PostgreSQL.
+ *
+ * Note: keep this in sync with the UserAuth enum in hba.h.
+ */
+static const char *const UserAuthName[] =
+{
+   "reject",
+   "implicit reject",          /* Not a user-visible option */
+   "trust",
+   "ident",
+   "password",
+   "md5",
+   "gss",
+   "sspi",
+   "pam",
+   "bsd",
+   "ldap",
+   "cert",
+   "radius",
+   "peer"
+};
Perhaps this could use a StaticAssertStmt()? Say something like that:
#define USER_AUTH_LAST uaPeer
StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,   "UserAuthName must include all user authentication
names");

Any updates could easily be forgotten.
-- 
Michael

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> On Mon, Jan 30, 2017 at 5:18 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>> #define USER_AUTH_LAST uaPeer
>> StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
>> "UserAuthName must include all user authentication names");

> Thanks for the review. Added the static assert statement.

This isn't exactly bulletproof, since somebody could add another enum
value and forget to update the macro.  Still, it's better than nothing.
I tried to make it a shade more idiot-proof by putting the #define
physically inside the enum list --- you'd have to really have blinders
on to not notice it there.  (Not that people haven't made equally silly
mistakes :-(.)

Pushed with that adjustment.  Thanks for working on this!
        regards, tom lane

On Tue, Jan 31, 2017 at 12:55 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
>
>
> On Tue, Jan 31, 2017 at 10:04 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> Haribabu Kommi <kommi.haribabu@gmail.com> writes:
>> > On Mon, Jan 30, 2017 at 5:18 PM, Michael Paquier
>> > <michael.paquier@gmail.com>
>> > wrote:
>> >> #define USER_AUTH_LAST uaPeer
>> >> StaticAssertStmt(lengthof(UserAuthName) == USER_AUTH_LAST + 1,
>> >> "UserAuthName must include all user authentication names");
>>
>> > Thanks for the review. Added the static assert statement.
>>
>> This isn't exactly bulletproof, since somebody could add another enum
>> value and forget to update the macro.  Still, it's better than nothing.
>> I tried to make it a shade more idiot-proof by putting the #define
>> physically inside the enum list --- you'd have to really have blinders
>> on to not notice it there.  (Not that people haven't made equally silly
>> mistakes :-(.)
>>
>> Pushed with that adjustment.  Thanks for working on this!
>
>
> Thanks for your support.

The modifications looks fine for me, thanks for adding the assertion.
-- 
Michael