Re: log_hostname and pg_stat_activity

Here is my review for this patch

Submission Review
----------------
-Patch applies cleanly

-Patch does not include documentation changes. At a minimum: update the
table that lists what pg_stat_activity and pg_stat_replication includes
in monitoring.sgml but I propose more below.

-No tests are included but writing unit tests that depend on produce the
same output involving the hostname of the client is not possible.

Usability review
----------------

See my comments below in the testing section. The patch does do what it
says but the log_hostname issue is a usability issue (it not being
obvious why you get only null owhen log_hostname is turned off).
Documenting it might be fine. If log_hostname were new to 9.1 I would
encourage renaming it to something that implies it does more than just
control logging output but I don't see this as a good enough reason to
rename a parameter.

I think being able to see the hostnames connections in pg_stat_activity
come from is useful and it is a feature we don't already have.

Feature Test
----------------

If my connection is authorized through a line in pg_hba that uses
client_hostname then the column shows what I expect even with
log_hostname set to off.

However if I connect with a line in pg_hba that matches on an IP network
then my client_hostname is always null unless log_hostname is set to
true. This is consistent with the behavior you describe but I think the
average user will find it a bit confusing. Having a column that is
always null unless a GUC is set is less than ideal but I understand why
log_hostname isn't on by default.

I would be inclined to add an entry for these views in catalogs.sgml
that where we can then give users a pointer that they need to set
log_hostname to get anything out of this column.

If I connect through unix sockets (say psql -h /tmp --port 5433)
I was also expecting to see "[local]" in client_hostname but I am just
getting NULL. This this lookup is static I don't see why it should be
dependent on log_hostname (even with log_hostname set I'm not seeing
[local])

I have not tested this with ipv6

Performance Review
------------------
The lookup is done when the connection is established not each time the
view is queried. I don't see any performance issues

Coding Review
-------------

As Alvaro pointed out BackendStatusShmemSize should be updated.

To answer his question about why clientaddr works: clientaddr is a
SockAddr which is a structure not a pointer so the data gets copied by
value to beentry. That won't work for strings, I have no issues with
how your allocating space in beentry and then strlcpy'ing the values.

I see no issues with the implementation

I'm marking this as 'Waiting for Author' pending documentation changes
and maybe a fix the behaviour with unix sockets

Steve