Re: SYSTEM_USER reserved word implementation

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Cc: Jacob Champion <jchampion@timescale.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Tom Lane <tgl@sss.pgh.pa.us>, Joe Conway <mail@joeconway.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-09-07T08:48:47Z
Lists: pgsql-hackers

Attachments

On Fri, Aug 26, 2022 at 10:02:26AM +0900, Michael Paquier wrote:
> FWIW, I was also wondering about the need for all this initialization
> stanza and the extra SystemUser in TopMemoryContext.  Now that we have
> MyClientConnectionInfo, I was thinking to just build the string in the
> SQL function as that's the only code path that needs to know about
> it.  True that this approach saves some extra palloc() calls each time
> the function is called.

At the end, fine by me to keep this approach as that's more
consistent.  I have reviewed the patch, and a few things caught my
attention:
- I think that we'd better switch InitializeSystemUser() to have two
const char * as arguments for authn_id and an auth_method, so as there
is no need to use tweaks with UserAuth or ClientConnectionInfo in
miscadmin.h to bypass an inclusion of libpq-be.h or hba.h.
- The OID of the new function  should be in the range 8000-9999, as
taught by unused_oids.
- Environments where the code is built without krb5 support would skip
the test where SYSTEM_USER should be not NULL when authenticated, so I
have added a test for that with MD5 in src/test/authentication/.
- Docs have been reworded, and I have applied an indentation.
- No need to use 200k rows in the table used to force the parallel
scan, as long as the costs are set.

It is a bit late here, so I may have missed something.  For now, how
does the attached look to you?
--
Michael

Commits

  1. Introduce SYSTEM_USER

  2. Add some information about authenticated identity via log_connections