From: | "Imseih (AWS), Sami" <simseih(at)amazon(dot)com> |
---|---|
To: | Maiquel Grassi <grassi(at)hotmail(dot)com(dot)br>, Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | 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-03-29 06:13:56 |
Message-ID: | 640B2586-EECF-44C0-B474-CA8510F8EAFC@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thank you for the updated patch.
First and foremost, thank you very much for the review.
> The initial and central idea was always to keep the metacommand
> "\conninfo" in its original state, that is, to preserve the string as it is.
> The idea of "\conninfo+" is to expand this to include more information.
> If I change the "\conninfo" command and transform it into a table,
> I would have to remove all the translation part (files) that has already
> been implemented in the past. I believe that keeping the string and
> its translations is a good idea and there is no great reason to change that.
You are right. Not much to be gained to change this.
For the current patch, I have a few more comments.
1/ The output should be reorganized to show the fields that are part of “conninfo” first.
I suggest it should look like this:
Current Connection Information
-[ RECORD 1 ]----------------+---------
Database | postgres
Authenticated User | postgres
Socket Directory | /tmp
Host |
Server Port | 5432
Server Address |
Client Address |
Client Port |
Backend PID | 30846
System User |
Current User | postgres
Session User | postgres
Application Name | psql
SSL Connection | f
SSL Protocol |
SSL Cipher |
SSL Compression |
GSSAPI Authenticated | f
GSSAPI Principal |
GSSAPI Encrypted | f
GSSAPI Credentials Delegated | f
With regards to the documentation. I think it's a good idea that every field has an
description. However, I have some comments:
1/ For the description of the conninfo command, what about simplifying like below?
"Outputs a string displaying information about the current database connection. When + is appended, more details about the connection are displayed in table format:"
2/ I don't think the descriptions need to start with "Displays". It is very repetitive.
3/ For the "Socket Directory" description, this could be NULL if the host was not specified.
what about the below?
"The socket directory of the connection. NULL if the host or hostaddr are specified for the connection"
4/ For most of the fields, they are just the output of a function, such as "pg_catalog.system_user()". What about the docs simply
link to the documentation of the function. This way we are not copying descriptions and have to be concerned if the description
of the function changes in the future.
5/ "true" and "false", do not need double quotes. This is not the convention used in other places docs.
Regards,
Sami
From | Date | Subject | |
---|---|---|---|
Next Message | Bertrand Drouvot | 2024-03-29 06:19:05 | Re: Introduce XID age and inactive timeout based replication slot invalidation |
Previous Message | Masahiko Sawada | 2024-03-29 06:05:36 | Re: [PoC] Improve dead tuple storage for lazy vacuum |