Re: Psql meta-command conninfo+

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

In response to

Responses

Browse pgsql-hackers by date

  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