Re: Psql meta-command conninfo+

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Maiquel Grassi <grassi(at)hotmail(dot)com(dot)br>
Cc: "Imseih (AWS), Sami" <simseih(at)amazon(dot)com>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>, Pavel Luzanov <p(dot)luzanov(at)postgrespro(dot)ru>, Erik Wienhold <ewie(at)ewie(dot)name>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Psql meta-command conninfo+
Date: 2024-04-01 16:22:41
Message-ID: 202404011622.eppdne4344lo@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello

Yeah, that's what I meant about the translations, thanks.

Two more comments,

- You don't need a two-level conditional here
if (pset.sversion >= 90500)
{
if (pset.sversion < 140000)
appendPQExpBuffer(&buf,
" ssl.ssl AS \"%s\",\n"
" ssl.version AS \"%s\",\n"
" ssl.cipher AS \"%s\",\n"
" ssl.compression AS \"%s\",\n",
_("SSL Connection"),
_("SSL Protocol"),
_("SSL Cipher"),
_("SSL Compression"));
if (pset.sversion >= 140000)
appendPQExpBuffer(&buf,
" ssl.ssl AS \"%s\",\n"
" ssl.version AS \"%s\",\n"
" ssl.cipher AS \"%s\",\n"
" NULL AS \"%s\",\n",
_("SSL Connection"),
_("SSL Protocol"),
_("SSL Cipher"),
_("SSL Compression"));
}
else
appendPQExpBuffer(&buf,
" NULL AS \"%s\",\n"
" NULL AS \"%s\",\n"
" NULL AS \"%s\",\n"
" NULL AS \"%s\",\n",
_("SSL Connection"),
_("SSL Protocol"),
_("SSL Cipher"),
_("SSL Compression"));

Instead you can just do something like

if (pset.version >= 140000)
one thing;
else if (pset.version > 90500)
second thing;
else
third thing;

This also appears where you add the GSSAPI columns; and it's also in the
final part where you append the FROM clause, though it looks a bit
different there.

- You have three lines to output a semicolon at the end of the query
based on version number. Remove the first two, and just have a final
one where the semicolon is added unconditionally.

- I don't think one <para> for each item in the docs is reasonable.
There's too much vertical whitespace in the output. Maybe do this
instead:

[...]
database connection. When <literal>+</literal> is appended,
more details about the connection are displayed in table
format:

<simplelist>
<member>
<term>Database:</term> The name of the current
database on this connection.
</member>

<member>
<term>Authenticated User:</term> The authenticated
user at the time of psql connection with the server.
</member>

...
</simplelist>

- This text is wrong to start with "Returns the":

System User: Returns the authentication method and the identity (if
any) that the user presented during the authentication cycle before
they were assigned a database role. It is represented as
auth_method:identity or NULL if the user has not been authenticated.

That minor point aside, I disagree with Sami about repeating the docs
for system_user() here. I would just say "The authentication data
provided for this connection; see the function system_user() for more
details." with a link to the appropriate section of the docs. Making
us edit this doc if we ever modify the behavior of the function is not
great.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"We're here to devour each other alive" (Hobbes)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-04-01 16:36:43 Re: Table AM Interface Enhancements
Previous Message Daniel Verite 2024-04-01 16:09:55 Re: psql's FETCH_COUNT (cursor) is not being respected for CTEs