Feedback-ID: ia2694551:Fastmail
Date: Mon, 1 Apr 2024 18:22:41 +0200
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Maiquel Grassi <grassi@hotmail.com.br>
Cc: "Imseih (AWS), Sami" <simseih@amazon.com>, 
	Nathan Bossart <nathandbossart@gmail.com>, Jim Jones <jim.jones@uni-muenster.de>, 
	Pavel Luzanov <p.luzanov@postgrespro.ru>, Erik Wienhold <ewie@ewie.name>, 
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Psql meta-command conninfo+
Message-ID: <202404011622.eppdne4344lo@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CP8P284MB24967A3224213796D569D4F2EC3F2@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
Archived-At: <https://www.postgresql.org/message-id/202404011622.eppdne4344lo%40alvherre.pgsql>
Precedence: bulk

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)