Hi,
On Wed, Jan 10, 2024 at 02:08:03PM +0100, Magnus Hagander wrote:
> On Wed, Jan 10, 2024 at 1:44 PM Aleksander Alekseev
> <aleksander@timescale.com> wrote:
> >
> > Hi,
> >
> > Thanks for the patch.
+1
> > > This overlaps with for example the values in pg_stat_gss, but it will
> > > include values for authentication methods that don't have their own
> > > view such as peer/ident. gss/ssl info will of course still be shown,
> > > it is just in more than one place.
Yeah, I think that's a good idea.
> > It hurts my sense of beauty that usename and authname are of different
> > types. But if I'm the only one, maybe we can close our eyes on this.
> > Also I suspect that placing usename and authname in a close proximity
> > can be somewhat confusing. Perhaps adding authname as the last column
> > of the view will solve both nitpicks?
>
> But it should probably actually be name, given that's the underlying
> datatype. I kept changing it around and ended up half way in
> between...
>
>
> > ```
> > + /* Information about the authenticated user */
> > + char st_authuser[NAMEDATALEN];
> > ```
> >
> > Well, here it's called "authuser" and it looks like the intention was
> > to use `name` datatype... I suggest using "authname" everywhere for
> > consistency.
I think it depends what we want the new field to reflect. If it is the exact
same thing as the SYSTEM_USER then I think it has to be text (as the SYSTEM_USER
is made of "auth_method:identity"). Now if we want it to be "only" the identity
part of it, then the `name` datatype would be fine. I'd vote for the exact same
thing as the SYSTEM_USER (means auth_method:identity).
> + <row>
> + <entry role="catalog_table_entry"><para role="column_definition">
> + <structfield>authname</structfield> <type>name</type>
> + </para>
> + <para>
> + The authentication method and identity (if any) that the user
> + used to log in. It contains the same value as
> + <xref linkend="system-user" /> returns in the backend.
> + </para></entry>
> + </row>
I'm fine with auth_method:identity.
> + S.authname,
What about using system_user as the field name? (because if we keep
auth_method:identity it's not really the authname anyway).
Also, what about adding a test in say 003_peer.pl to check that the value matches
the SYSTEM_USER one?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
