Re: First draft of PG 17 release notes

[Maiquel Grassi]

Mhd

Enviado desde Outlook para Android

From: Bruce Momjian 
Sent: Friday, May 10, 2024 4:47:04 PM
To: Daniel Verite 
Cc: PostgreSQL-development 
Subject: Re: First draft of PG 17 release notes

On Fri, May 10, 2024 at 06:29:11PM +0200, Daniel Verite wrote:
>Bruce Momjian wrote:
>
> >  have committed the first draft of the PG 17 release notes;  you can
> > see the results here:
> >
> > https://momjian.us/pgsql_docs/release-17.html
>
> In the psql items, I'd suggest mentioning
>
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=90f5178
>
> For the short description, maybe something like that:
>
> - Improve FETCH_COUNT to work with all queries (Daniel Vérité)
> Previously, results would be fetched in chunks only for queries
> that start with the SELECT keyword.

Agreed, patch attached and applied.

--
  Bruce Momjian  https://momjian.us
  EDB  https://enterprisedb.com

  Only you can decide what is important to you.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi!

(v29)

I left the command \conninfo in its original form.
I removed the 'Application Name' column from the \conninfo+ command.

Thanks,
Maiquel Grassi.


v29-0001-psql-meta-command-conninfo-plus.patch
Description: v29-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>> The existing \conninfo command gets its values from libpq APIs.  You are
>> changing all of this to make a server query, which is a totally
>> different thing.  If we wanted to take a change like this, I don't think
>> it should be reusing the \conninfo command.

FWIW, I agree with Peter's concern here, for a couple of reasons:

* If \conninfo involves a server query, then it will fail outright
if the server is in an aborted transaction, or if there's something
wrong with the connection --- which seems like one of the primary
cases where you'd wish to use \conninfo.

* What if there's a discrepancy between what libpq thinks and what
the server thinks?  (This is hardly unlikely for, say, host names.)
In the current situation you can use \conninfo to investigate the
client-side parameters and then use a manual query to see what the
server thinks.  If we replace \conninfo with a server query then
there is no way at all to verify libpq parameters from the psql
command line.

So I concur that \conninfo should continue to report on libpq values
and nothing else.  We can certainly talk about expanding it within
that charter, if there's useful stuff it doesn't show now.  But a
command that retrieves settings from the server should be a distinct
thing.

---//---

Well, I can revert \conninfo to its original state and keep \conninfo+
as it is, perhaps removing the application name, as I believe everything
else is important for a DBA/SysAdmin. Do you think we can proceed
with the patch this way? I am open to ideas that make \conninfo not
limited to just four or five basic pieces of information and easily bring
everything else to the screen.

Regards,
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

The existing \conninfo command gets its values from libpq APIs.  You are
changing all of this to make a server query, which is a totally
different thing.  If we wanted to take a change like this, I don't think
it should be reusing the \conninfo command.

But I don't really see the point of this.  The information you are
querying is already available in various system views.  This proposal is
just a shorthand for a collection of various random things some people
like to see.  Like, by what reason is application name included as
connection info?  Why not any other session settings?  What about
long-term maintenance: By what logic should things be added to this?

--//--

Hello Peter, thank you for your participation.
No, "they are not random things that random people like to see", that's not 
true.
Have you read the entire conversation from the beginning? We have already
discussed it a bit and I have provided an explanation about the motivation to
implement this expansion of the "\conninfo" command. The thing is, if you
have really been or worked as a DBA on a daily basis, taking care of many
databases and PostgreSQL clusters, something like this additional command
is the shortcut that every DBA needs. The application name was a suggestion
from a member. If you think it's not necessary, we can remove it. Furthermore,
if you believe that the patch is not well implemented, you, being a PostgreSQL 
guru,
tell me how I can improve the current patch and we move towards v29. I'm not in
a hurry, I just want it to be implemented in the best possible shape.

Best regards,
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi folks,

(v28)

I made a modification to a variable in the 'host' column in the archive 
'describe.c'.

Test:

[postgres@localhost ~]$ /home/pgsql-17devel/bin/psql -x -p 5432
psql (17devel, server 15.6)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
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  | 26728
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 |

postgres=# \q
[postgres@localhost ~]$ /home/pgsql-17devel/bin/psql -x -p 5000
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5000".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]+-
Database | postgres
Authenticated User   | postgres
Socket Directory | /tmp
Host |
Server Port  | 5000
Server Address   |
Client Address   |
Client Port  |
Backend PID  | 26730
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

Regards,
Maiquel Grassi.


v28-0001-psql-meta-command-conninfo-plus.patch
Description: v28-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Building the docs fail for v26. The error is:



ref/psql-ref.sgml:1042: element member: validity error : Element term is not 
declared in member list of possible children

 

  ^



I am able to build up to v24 before the  was replaced with 




I tested building with a modified structure as below; the  is a 

that has a  within it.  Each field is a simple list member and

the the name of the fields should be .



  

   \conninfo[+]

   

   

Outputs a string displaying information about the current

database connection. When + is appended,

more details about the connection are displayed in table

format:

   

   Database:The database name of the 
connection.

   Authenticated User:The authenticated database 
user of the connection.

   Current User:The user name of the current 
execution context;

  see the current_user() function in

   for more 
details.

   

   

   

  

---//---

Hi Sami!
(v27)
I made the adjustment in the documentation.
Thank you for the time dedicated to this feature.
Regards,
Maiquel Grassi.


v27-0001-psql-meta-command-conninfo-plus.patch
Description: v27-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hello Sami,

(v26)

>  Here is an example [1] where the session information functions are 
> referenced.

>  The public doc for this example is in [2].


>  Something similar can be done here. For example:


>  The session user's name; see the session_user() 
> function in

>   for more details.

I updated the patch.

Thank you for this help.

Regards,
Maiquel Grassi.


v26-0001-psql-meta-command-conninfo-plus.patch
Description: v26-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Amigos, boa tarde!

(v25)

>if (pset.version >= 14)
>  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.

Adjustment made.

>  - I don't think one  for each item in the docs is reasonable.
>  There's too much vertical whitespace in the output.  Maybe do this
>  instead:
>
>[...]
>database connection. When + is appended,
>more details about the connection are displayed in table
>format:
>
>
> 
>  Database: The name of the current
>  database on this connection.
> 
>
> 
>   Authenticated User: The authenticated
>   user at the time of psql connection with the server.
> 
>
> ...
>

Adjustment made. But I think it needs a review glance.

>  - 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.

Here I considered your suggestion (Sami and Álvaro's). However, I haven't yet
added the links for the functions system_user(), current_user(), and 
session_user().
I'm not sure how to do it. Any suggestion on how to create/add the link?

Regards,
Maiquel Grassi.


v25-0001-psql-meta-command-conninfo-plus.patch
Description: v25-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Database:

The database name of the connection.



Authenticated User:

The authenticated database user of the connection.



Socket Directory:

The Unix socket directory of the connection. NULL if host or hostaddr are 
specified.



Host:

The host name or address of the connection. NULL if a Unix socket is used.



Server Port:

The IP address of the connection. NULL if a Unix socket is used.



Server Address:

The IP address of the host name. NULL if a Unix socket is used.



Client Address:

The IP address of the client connected to this backend. NULL if a Unix socket 
is used.



Client Port:

The port of the client connected to this backend. NULL if a Unix socket is used.



Backend PID:

The process id of the backend for the connection.



Application name:

psql is the default for a psql connection

unless otherwise specified in the 

configuration parameter.



-//-

Hi Sami,
I considered your suggestions and Álvaro's suggestions
Regards,
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi!

(v24)

> I meant those columns to be just examples, but this should be applied to
> all the other columns in the output as well.

Applied the translation to all columns.

Regards,
Maiquel Grassi.


v24-0001-psql-meta-command-conninfo-plus.patch
Description: v24-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Note that, in the patch as posted, the column names are not
translatable.  In order to be translatable, the code needs to do
something like

appendPQExpBuffer(,
"  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 of

   appendPQExpBuffer(,
 "  NULL AS \"SSL Connection\",\n"
 "  NULL AS \"SSL Protocol\",\n"
 "  NULL AS \"SSL Cipher\",\n"
 "  NULL AS \"SSL Compression\",\n");


Please list me as reviewer for this patch, as I provided significant
guidance before it was even posted.

-//-

Hello!

(v23)

Yes Álvaro, I have already appointed you as the patch reviewer.
It's true, even before publishing it on Commifest, you have
already provided good ideas and guidance.

I adjusted the translation syntax for the SSL and GSSAPI columns.
After your validation, that is, an okay confirmation that it's fine, I'll
proceed with the others.

Test:

[postgres@localhost bin]$ ./psql -x -p 5000 -h 127.0.0.1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5000".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]+--
Database | postgres
Authenticated User   | postgres
Socket Directory |
Host | 127.0.0.1
Server Port  | 5000
Server Address   | 127.0.0.1
Client Address   | 127.0.0.1
Client Port  | 52966
Backend PID  | 1693
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

Regards,
Maiquel Grassi.


v23-0001-psql-meta-command-conninfo-plus.patch
Description: v23-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>  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.

>  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.

-//-

Hi Sami!

(v22)


I did everything you mentioned earlier, that is, I followed all your 
suggestions. However,
I didn't complete item 4. I'm not sure, but I believe that linking it to the 
documentation
could confuse the user a bit. I chose to keep the descriptions as they were. 
However, if
you have any ideas on how we could outline it, let me know and perhaps we can
implement it.

Thank you so much!

Exemples:

[postgres@localhost bin]$ ./psql -x -p 5000 -h 127.0.0.1

psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5000".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]+--
Database | postgres
Authenticated User   | postgres
Socket Directory |
Host | 127.0.0.1
Server Port  | 5000
Server Address   | 127.0.0.1
Client Address   | 127.0.0.1
Client Port  | 33100
Backend PID  | 2974
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

postgres=# \q
[postgres@localhost bin]$ ./psql -x -p 5432 -h localhost
Password for user postgres:
psql (17devel, server 15.6)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]+--
Database | postgres
Authenticated User   | postgres
Socket Directory |
Host | localhost
Server Port  | 5432
Server Address   | ::1
Client Address   | ::1
Client Port  | 57010
Backend PID  | 3000
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 |


Regards,
Maiquel Grassi.


v22-0001-psql-meta-command-conninfo-plus.patch
Description: v22-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi Sami!

(v21)

First and foremost, thank you very much for the review.


> 1/ I looked through other psql-meta commands and the “+” adds details but

> does not change output format. In this patch, conninfo and conninfo+

> have completely different output. The former is a string with all the details

> and the latter is a table. Should conninfo just be a table with the minimal 
> info

> and additional details can be displayed with "+" appended?

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.

> 2/ GSSAPI details should show the full details, such as "principal",

> "encrypted" and "credentials_delegated".

In this new patch, I added these columns. I handled the 'server versions'
for cases where the 'credentials_delegated' column is not in the view.
It turned out well. Thank you for the idea.

3/ A very nice to have is "Application Name", in the case one

sets the application_name within a connection or in the connection string.

I did the same here. I liked your idea and added this column in the SQL.

Look below to see how it turned out after it's finished.

Exemple 1:

[postgres@localhost ~]$ /home/pgsql-17devel/bin/psql -x -p 5432

psql (17devel, server 15.6)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]+-
Database | postgres
Authenticated User   | postgres
System User  |
Current User | postgres
Session User | postgres
Application Name | psql
Backend PID  | 19439
Server Address   |
Server Port  | 5432
Client Address   |
Client Port  |
Socket Directory | /tmp
Host |
SSL Connection   | f
SSL Protocol |
SSL Cipher   |
SSL Compression  |
GSSAPI Authenticated | f
GSSAPI Principal |
GSSAPI Encrypted | f
GSSAPI Credentials Delegated |

Exemple 2:

[postgres@localhost ~]$ /home/pgsql-17devel/bin/psql -x -p 5000
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5000".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]+-
Database | postgres
Authenticated User   | postgres
System User  |
Current User | postgres
Session User | postgres
Application Name | psql
Backend PID  | 19468
Server Address   |
Server Port  | 5000
Client Address   |
Client Port  |
Socket Directory | /tmp
Host |
SSL Connection   | f
SSL Protocol |
SSL Cipher   |
SSL Compression  |
GSSAPI Authenticated | f
GSSAPI Principal |
GSSAPI Encrypted | f
GSSAPI Credentials Delegated | f

Regards,
Maiquel Grassi.


v21-0001-psql-meta-command-conninfo-plus.patch
Description: v21-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

> Hi Nathan!

>
> Sorry for the delay, I was busy with other professional as well as personal 
> activities.

> I made all the changes you suggested. I removed the variables and started 
> using
> views "pg_stat_ssl" and "pg_stat_gssapi". I handled the PostgreSQL versioning 
> regarding the views used.

--//--

Olá Nathan!

I think we are very close to possibly finishing this patch, and
I would like your help to do it. Is there any correction you
deem necessary still?

Regards,
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

On Thu, Feb 29, 2024 at 10:02:21PM +, Maiquel Grassi wrote:
> Sorry for the delay. I will make the adjustments as requested soon.

Looking forward to it.

//

Hi Nathan!

Sorry for the delay, I was busy with other professional as well as personal 
activities.

I made all the changes you suggested. I removed the variables and started using
views "pg_stat_ssl" and "pg_stat_gssapi". I handled the PostgreSQL versioning 
regarding the views used.

Here's a brief demonstration of the result:

[postgres@localhost ~]$ /home/pgsql-17devel/bin/psql -E -x -p 5433

psql (17devel)
Type "help" for help.

postgres=# \conninfo+
/ QUERY */
SELECT
  pg_catalog.current_database() AS "Database",
  'postgres' AS "Authenticated User",
  pg_catalog.system_user() AS "System User",
  pg_catalog.current_user() AS "Current User",
  pg_catalog.session_user() AS "Session User",
  pg_catalog.pg_backend_pid() AS "Backend PID",
  pg_catalog.inet_server_addr() AS "Server Address",
  pg_catalog.current_setting('port') AS "Server Port",
  pg_catalog.inet_client_addr() AS "Client Address",
  pg_catalog.inet_client_port() AS "Client Port",
  '/tmp' AS "Socket Directory",
  CASE
WHEN
  pg_catalog.inet_server_addr() IS NULL
  AND pg_catalog.inet_client_addr() IS NULL
THEN NULL
ELSE '/tmp'
  END AS "Host",
  (SELECT gss_authenticated AS "GSSAPI"
  FROM pg_catalog.pg_stat_gssapi
  WHERE pid = pg_catalog.pg_backend_pid()),
  ssl.ssl AS "SSL Connection",
  ssl.version AS "SSL Protocol",
  ssl.cipher AS "SSL Cipher",
  NULL AS "SSL Compression"
FROM
  pg_catalog.pg_stat_ssl ssl
WHERE
  pid = pg_catalog.pg_backend_pid()
;
//

Current Connection Information
-[ RECORD 1 ]--+-
Database   | postgres
Authenticated User | postgres
System User|
Current User   | postgres
Session User   | postgres
Backend PID| 29007
Server Address |
Server Port| 5433
Client Address |
Client Port|
Socket Directory   | /tmp
Host   |
GSSAPI | f
SSL Connection | f
SSL Protocol   |
SSL Cipher |
SSL Compression|

Rergards,
Maiquel Grassi.


v20-0001-psql-meta-command-conninfo-plus.patch
Description: v20-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

On Sat, Feb 17, 2024 at 02:53:43PM +, Maiquel Grassi wrote:
>> The "pg_stat_ssl" view is available from >= PG 9.5, and the "pg_stat_gssapi" 
>> view is
>> available from >= PG 12. The "compression" column was removed from the
>> "pg_stat_ssl" from >= PG 14. All of these cases introduce greater complexity 
>> in
>> maintaining the SQL. The central idea from the beginning has always been to 
>> show
>> the user all the information from \conninfo and extend it in \conninfo+.

>IMHO we should use the views whenever possible (for the reason stated
>above [0]).  I think it's okay if we need to fall back to a different
>approach for older versions.  But presumably we'll discontinue psql support
>for these old server versions at some point, at which point we can simply
>delete the dead code that doesn't use the views.

>> The absence
>> of the "compression" column in version 14 and above makes dealing with this 
>> even
>> more complicated, and not showing it seems to contradict \conninfo.

>I would be okay with using PQsslAttribute() for all versions for this one
>since any remaining support for this feature is on its way out.  Once psql
>no longer supports any versions that allow SSL compression, we could
>probably remove it from \conninfo[+] completely or hard-code it to "off".

>> SSL support has been available since version 7.1 (see documentation); if 
>> there was
>> support before that, I can't say. In this regard, it may seem strange, but 
>> there are still
>> many legacy systems running older versions of PostgreSQL. Just yesterday, I 
>> assisted
>> a client who is still using PG 8.2. In these cases, using the "pg_stat_ssl" 
>> and
>> "pg_stat_gssapi" views would not be possible because they don't exist on the 
>> server.
>> I believe that psql should cover as many cases as possible when it comes to 
>> compatibility
>> with older versions (even those no longer supported). In this case, 
>> concerning SSL and
>> GSS, I think libpq is better prepared to handle this.

>At the moment, the psql support cutoff appears to be v9.2 (see commit
>cf0cab8), which has been out of support for over 6 years.  If \conninfo+
>happens to work for older versions, then great, but I don't think we should
>expend too much energy in this area.

[0] https://postgr.es/m/20240216155449.GA1236054%40nathanxps13

//

Hi Nathan!

Sorry for the delay. I will make the adjustments as requested soon.

Regards,
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi!

(v19)

Adjusted the description of each column in the documentation as promised.
I used the existing documentation as a basis for each SQL function used,
as well as for functions and views related to SSL and GSSAPI (documentation).

If you can validate, I appreciate it.

Regards,
Maiquel Grassi.


v19-0001-psql-meta-command-conninfo-plus.patch
Description: v19-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi Nathan!

(v18)

>I might be alone on this, but I think this command should output the same
>columns regardless of the version, whether it's using SSL, etc. and just
>put NULL in any that do not apply. IMHO that would simplify the code and
>help prevent confusion. Plus, I'm not aware of any existing meta-commands
>that provide certain columns conditionally.

I implemented your suggestion. Now, whenever the \conninfo+ command is
invoked, all columns will appear. Contextually inappropriate cases will return
NULL. I also adjusted the names of the columns related to SSL to make this
information clearer for the user. I haven't focused on documenting the
columns yet. I will do that soon.

>Could we pull some of this information from pg_stat_ssl instead of from
>libpq? The reason I suggest this is because I think it would be nice if
>the query that \conninfo+ uses could be copy/pasted as needed and not rely
>on hard-coded values chosen by the client.

I've been considering using the views "pg_stat_ssl" and "pg_stat_gssapi"; 
however,
I realized that dealing with version-related cases using them is more 
complicated.

Let me explain the reasons:

The "pg_stat_ssl" view is available from >= PG 9.5, and the "pg_stat_gssapi" 
view is
available from >= PG 12. The "compression" column was removed from the
"pg_stat_ssl" from >= PG 14. All of these cases introduce greater complexity in
maintaining the SQL. The central idea from the beginning has always been to show
the user all the information from \conninfo and extend it in \conninfo+. The 
absence
of the "compression" column in version 14 and above makes dealing with this even
more complicated, and not showing it seems to contradict \conninfo.


SSL support has been available since version 7.1 (see documentation); if there 
was

support before that, I can't say. In this regard, it may seem strange, but 
there are still
many legacy systems running older versions of PostgreSQL. Just yesterday, I 
assisted
a client who is still using PG 8.2. In these cases, using the "pg_stat_ssl" and
"pg_stat_gssapi" views would not be possible because they don't exist on the 
server.
I believe that psql should cover as many cases as possible when it comes to 
compatibility
with older versions (even those no longer supported). In this case, concerning 
SSL and
GSS, I think libpq is better prepared to handle this.

I may be mistaken in my statement and I welcome any better suggestions. For 
now, I've
maintained the implementation using libpq as it seems to be working well and is 
not
contradicting \conninfo. If you have any suggestions on how to work around the 
absence
of the "compression" column, we can reconsider how to implement it without 
using libpq.

Tests:

[postgres@localhost bin]$ ./psql -x -h 127.0.0.1 -p 5432

Password for user postgres:
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+---
Database   | postgres
Authenticated User | postgres
System User| scram-sha-256:postgres
Current User   | postgres
Session User   | postgres
Backend PID| 22431
Server Address | 127.0.0.1
Server Port| 5432
Client Address | 127.0.0.1
Client Port| 51300
Socket Directory   |
Host   | 127.0.0.1
SSL Connection | f
SSL Protocol   |
SSL Cipher |
SSL Compression|
GSSAPI | f


[postgres@localhost bin]$ ./psql -x -h 127.0.0.1 -p 5433
Password for user postgres:
psql (17devel, server 15.6)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, 
compression: off)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5433".
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, 
compression: off)
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+
Database   | postgres
Authenticated User | postgres
System User|
Current User   | postgres
Session User   | postgres
Backend PID| 22438
Server Address | 127.0.0.1
Server Port| 5433
Client Address | 127.0.0.1
Client Port| 36016
Socket Directory   |
Host   | 127.0.0.1
SSL Connection | t
SSL Protocol   | TLSv1.2
SSL Cipher | ECDHE-RSA-AES256-GCM-SHA384
SSL Compression| off
GSSAPI | f

Thank you very much for your sugestions and help!
Maiquel Grassi.


v18-0001-psql-meta-command-conninfo-plus.patch
Description: v18-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi!

>7) -h 0.0.0.0 - As you mentioned, this is one of the cases where host
>and "server address" differ.
>   I am not sure if it is an issue. Perhaps the other reviewers might
>have an opinion on it

>$ /usr/local/postgres-dev/bin/psql -x -U postgres -h 0.0.0.0 -c
>"\conninfo+" -c "\conninfo"
>Current Connection Information
>-[ RECORD 1 ]--+---
>Database   | postgres
>Authenticated User | postgres
>System User|
>Current User   | postgres
>Session User   | postgres
>Backend PID| 404395
>Server Address | 127.0.0.1
>Server Port| 5432
>Client Address | 127.0.0.1
>Client Port| 54806
>Socket Directory   |
>Host   | 0.0.0.0
>Encryption | SSL
>Protocol   | TLSv1.3
>Cipher | TLS_AES_256_GCM_SHA384
>Compression| off

I believe we don't actually have a problem here. To me, this makes sense.
I'm not a Networking expert, but from my standpoint, any string or IP
accepted by the "host" parameter of psql, pointing to "127.0.0.1" or to "::1", 
is correct.
I see it as a convention (would need to revisit Tanenbaum's books haha),
and I don't think there's a need to modify it. But as you mentioned, if someone
with more Networking knowledge could weigh in, a suggestion would be welcome.

>I'm not sure of the impact of this change in the existing \conninfo - at
>least the cfbot and "make -j check-world" didn't complain.
>I'll take a closer look at it as soon we have test cases.

This type of modification, in principle, does not generate errors as we
are only changing the display on screen of the strings already previously 
stored.

>To keep it consistent with the other options, we might wanna use "+ is
>appended" instead of "+ is specified" or "+ is given"

It seems to me that "appended" sounds good. I opted for it.
Following your suggestion, and merging this with the existing
previous ideas from the documentation, I've created a provisional
prototype of the column descriptions. At a later stage, I'll place the
descriptions correctly by removing placeholders (blah blah blah).
Along with this, I'll evaluate an iteration suggested by Nathan Bossart,

who also proposed changes.

Thank you for your work.

Regards,
Maiquel Grassi.


v17-0001-psql-meta-command-conninfo-plus.patch
Description: v17-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi!

(v16)

In this version, I made a small adjustment to the indentation
of the \conninfo code and described the columns as returned
by \conninfo+ as suggested by Jim Jones.

Regards,
Maiquel Grassi.


v16-0001-psql-meta-command-conninfo-plus.patch
Description: v16-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

() function in 
"address".

Result:

[postgres@localhost bin]$ ./psql -x hostaddr=0

psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "0" 
(address "127.0.0.1") at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+--
Database   | postgres
Authenticated User | postgres
System User|
Current User   | postgres
Session User   | postgres
Backend PID| 26859
Server Address | 127.0.0.1
Server Port| 5432
Client Address | 127.0.0.1
Client Port| 58254
Socket Directory   |
Host   | 0

//

>I think the documentation could add a little more info than just this:

>> When no + is specified, it simply prints a textual description of a
>>few connection options. When + is given, more complete information is
>>displayed as a table.

>Perhaps briefly mentioning the returned columns or simply listing them
>would be IMHO a nice addition. For some users the semantics of
>"Authenticated User", "System User", "Current User" and "Session User"
>can be a little confusing. And yes, I realize the current documentation
>of \conninfo is already a little vague :).

Your suggestion was well received, and I'will made the adjustment to make
the command description more comprehensive.

Here is version v15 where I sought to correct 'Adress' to make it the same
as 'Server Address'.

Could you perform the same test and validate?

Thank you so much!
Maiquel Grassi.


v15-0001-psql-meta-command-conninfo-plus.patch
Description: v15-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>I found that \conninfo and \conninfo+ act differently when the connection is 
>broken.
>I used pg_terminate_backend function from another session to terminate an open 
>psql session.
>After that, \conninfo does not see the connection break (surprisingly!), and 
>\conninfo+ returns an error:
>
>postgres@postgres(17.0)=# \conninfo+
>FATAL:  terminating connection due to administrator command
>server closed the connection unexpectedly
>   This probably means the server terminated abnormally
>   before or while processing the request.
>The connection to the server was lost. Attempting reset: Succeeded.
>
>postgres@postgres(17.0)=# \conninfo
>You are connected to database "postgres" as user "postgres" via socket in 
>"/tmp" at port "5401".
>
>Another surprise is that this check: if (db == NULL) did not work in both 
>cases.

--//--

Hi Pavel!

(v14)

The "if (db == NULL)" has been removed, as well
as the message inside it. To always maintain the
same error messages, I changed the validation of
"\conninfo" to match the validation used for "\conninfo+".
Therefore, now "\conninfo" and "\conninfo+" return
the same error when "pg_terminate_backend()" terminates
this session through another session. The result of
the adjustment is as follows:


Case 1 ("\conninfo+"):

[postgres@localhost bin]$ ./psql -x
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+-
Database   | postgres
Authenticated User | postgres
System User|
Current User   | postgres
Session User   | postgres
Backend PID| 24381
Server Address |
Server Port| 5432
Client Address |
Client Port|
Socket Directory   | /tmp
Host   |

postgres=# \conninfo+
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.


Case 2 ("\conninfo"):

[postgres@localhost bin]$ ./psql -x
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+-
Database   | postgres
Authenticated User | postgres
System User|
Current User   | postgres
Session User   | postgres
Backend PID| 24539
Server Address |
Server Port| 5432
Client Address |
Client Port|
Socket Directory   | /tmp
Host   |

postgres=# \conninfo
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.

The connection to the server was lost. Attempting reset: Succeeded.

In both cases, the sessions were terminated by another session.

Regards,
Maiquel Grassi.


v14-0001-psql-meta-command-conninfo-plus.patch
Description: v14-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>Database   | postgres
>[...]
>Host   | 127.0.0.1
>Encryption | SSL
>Protocol   | PQsslAttribute(protocol)
>Cipher | PQsslAttribute(cipher)
>Compression| PQsslAttribute(compression)
>
>When GSS, like this
>
>Database   | postgres
>[...]
>Host   | 127.0.0.1
>Encryption | GSS

--//--

Hi PgHackers,

Columns were added for SSL and GSS.


For SSL, I conducted some tests as follows. For GSS, I will perform
them and intend to provide a sample here in the next interaction.

If anyone can and wants to test GSSAPI as well, I appreciate it.

[postgres@localhost bin]$ ./psql -h localhost -p 5432 -x

psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+--
Database   | postgres
Authenticated User | postgres
System User|
Current User   | postgres
Session User   | postgres
Backend PID| 15809
Server Address | ::1
Server Port| 5432
Client Address | ::1
Client Port| 56890
Socket Directory   |
Host   | localhost

postgres=# \q
[postgres@localhost bin]$ ./psql -h localhost -p 5433 -x
psql (17devel, server 15.6)
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, 
compression: off)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5433".
SSL connection (protocol: TLSv1.2, cipher: ECDHE-RSA-AES256-GCM-SHA384, 
compression: off)
postgres=# \conninfo+
Current Connection Information
-[ RECORD 1 ]--+
Database   | postgres
Authenticated User | postgres
Current User   | postgres
Session User   | postgres
Backend PID| 15811
Server Address | ::1
Server Port| 5433
Client Address | ::1
Client Port| 40622
Socket Directory   |
Host       | localhost
Encryption | SSL
Protocol   | TLSv1.2
Cipher | ECDHE-RSA-AES256-GCM-SHA384
Compression| off

Regards,
Maiquel Grassi.


v13-0001-psql-meta-command-conninfo-plus.patch
Description: v13-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>Hmm, I noticed that this would call printSSLInfo() and printGSSInfo()
>after listConnectionInformation.  It would be much better to show these
>in tabular format as well and remove the calls to printSSL/GSSInfo.
>
>I propose additional columns to the same \conninfo+ table; when SSL,
>like this:
>
>Database   | postgres
>[...]
>Host   | 127.0.0.1
>Encryption | SSL
>Protocol   | PQsslAttribute(protocol)
>Cipher | PQsslAttribute(cipher)
>Compression| PQsslAttribute(compression)
>
>When GSS, like this
>
>Database   | postgres
>[...]
>Host   | 127.0.0.1
>Encryption | GSS
>
>(why don't we print anything else in printGSSInfo()?  That's weird.)

--//--

Hi Álvaro,


Thank you for the suggestion. I will remove printSSLInfo() and printGSSInfo() 
and
incorporate the suggested fields into the tabular result of "\conninfo+".

If it's necessary to adjust printGSSInfo(), we can work on that as well. At 
first
glance, I leave it open for others to analyze and give their opinion.

I'd also like to ask for help with a difficulty. Currently, I'm working on

resolving this situation (highlighted by Pavel Luzanov). How can we
make \conninfo return the same message as \conninfo+ after closing
the current session in another session with pg_terminate_backend(pid)?

[postgres@localhost bin]$ ./psql

psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
 
Current Connection Information
 Database | Authenticated User | System User | Current User | Session User | 
Backend PID | Server Address | Server Port | Client Address | Client Port | 
Socket Directory | Host
--++-+--+--+-++-++-+--+--
 postgres | postgres   | | postgres | postgres |
   17281 || 5432|| | /tmp   
  |
(1 row)

postgres=# 2024-02-09 09:15:24.152 -03 [17281] FATAL:  terminating connection 
due to administrator command

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
FATAL:  terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
postgres=#

Tks a lot!
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>Hi Nathan
>
>On 09.02.24 03:41, Nathan Bossart wrote:
>
>> Separately, does
>> the server version really belong here?  I'm not sure I would consider that
>> to be connection information.
>>
>I tend to agree with you. The info there wouldn't hurt, but perhaps the
>client version would be a better fit.

--//--

Hi!

I believe that if we include the server version, it would also be
necessary to include the version of psql. This would make it
clearer for the user. I agree that this won't make much difference
in practice, especially because we're interested in creating a setof
information directly related to the connection. I prefer to keep it
suppressed for now. In the future, if necessary, we can add this
information without any problem. In v12, "Server Version" is
already removed.

Tks!
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

Hi Pavel!

>The patch v10 build ends with a warning:
>$ make -j --silent
>describe.c:911:1: warning: no previous prototype for 
>‘listConnectionInformation’ [-Wmissing-prototypes]
>  911 | listConnectionInformation()
  | ^

>About terms.

I resolved this in v11. I had forgotten to put
the 'void' inside the parentheses (describe.h and describe.c).


>postgres@postgres(17.0)=# \x \conninfo+
>Expanded display is on.
>Current Connection Information
>-[ RECORD 1 ]--+-
>Database   | postgres
>Authenticated User | postgres
>System User|
>Current User   | postgres
>Session User   | postgres
>Session PID| 951112
>Server Version | 17devel
>Server Address |
>Server Port| 5401
>Client Address |
>Client Port|
>Socket Directory   | /tmp
>Host   |

>It looks like "Session PID" is a new term for the server process identifier.
>How about changing the name to "Backend PID" (from pg_backend_pid) or even PID 
>(from pg_stat_activity)?


You're right, it's better to stick with the proper terms already in use
in PostgreSQL. This ensures that the user doesn't get confused. I've
changed it to "Backend PID".


>On 08.02.2024 17:58, Maiquel Grassi wrote:

> 1.
> + if (db == NULL)
> + printf(_("You are currently not connected to a 
> database.\n"));
>
> This check is performed for \conninfo, but not for \conninfo+.

1. The connection check for the case of \conninfo+ is handled by "describe.c" 
itself since it deals with queries. I might be mistaken, but I believe that by 
using "printQuery()" via "describe.c", this is already ensured, and there is no 
need to evaluate the connection status.

>I found that \conninfo and \conninfo+ act differently when the connection is 
>broken.
>I used pg_terminate_backend function from another session to terminate an open 
>psql session.
>After that, \conninfo does not see the connection break (surprisingly!), and 
>\conninfo+ returns an error:

>postgres@postgres(17.0)=# \conninfo+
>FATAL:  terminating connection due to administrator command
>server closed the connection unexpectedly
>This probably means the server terminated abnormally
>before or while processing the request.
>The connection to the server was lost. Attempting reset: Succeeded.


For this case, I believe it's already resolved, because if you get a
return indicating that the connection was terminated, and indeed it was,
then "describe.c" is handling it correctly. At least that's what
it seems like.

>postgres@postgres(17.0)=# \conninfo
>You are connected to database "postgres" as user "postgres" via socket in 
>"/tmp" at port "5401".


Here it seems like we have an issue to be studied and subsequently resolved.

>Another surprise is that this check: if (db == NULL) did not work in both 
>cases.

I will investigate further and, if necessary, remove it.

Here's v12, in the next version, I'll try to address the above situation.

Thanks a lot!
Maiquel Grassi.


v12-0001-psql-meta-command-conninfo-plus.patch
Description: v12-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>Sorry if this has been brought up, but I noticed that some of the

>information is listed below the result set:
>
  >  postgres=# \conninfo+
  >Current Connection Information
  >  -[ RECORD 1 ]--+-
  > Database   | postgres
  >  Authenticated User | nathan
  >  System User|
  >  Current User   | nathan
  >  Session User   | nathan
  > Session PID| 659410
  >  Server Version | 17devel
  >  Server Address | ::1
  >  Server Port| 5432
  >  Client Address | ::1
  >  Client Port| 59886
  >  Socket Directory   |
  >  Host   | ::1

  >  SSL connection (protocol: TLSv1.3, cipher: TLS_AES_256_GCM_SHA384, 
compression: off)

>Shouldn't we move this information into the result set?  Separately, does
>the server version really belong here?  I'm not sure I would consider that
>to be connection information.

--//--


Hi Nathan,

The "Server Version" information is closely related to the connection. However,
it does seem to be an element that does not belong to this set. I removed this
column and left only what is truly connection info.


Regarding the functions "printSSLInfo()" and "printGSSInfo()", I agree that we
should include them in the returned dataset (as two additional columns). A good
argument is that this will make more sense when using \x (Expanded display).
However, they are declared and defined within the "command.c" file. To make
calls to "printSSLInfo()" and "printGSSInfo()" within "describe.c", we would 
need
to move their declarations to a new header and create a new C file. I believe
something like "ssl_gss_info.h" and "ssl_gss_info.c". I'm not sure, but at 
first glance,
this is what occurs to me. Do you have any better or more concise suggestions
for resolving this?

Regards,
Maiquel Grassi.


v11-0001-psql-meta-command-conninfo-plus.patch
Description: v11-0001-psql-meta-command-conninfo-plus.patch


v11-0001-psql-meta-command-conninfo-plus.patch
Description: v11-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

On 08.02.24 21:37, Erik Wienhold wrote:
>> Modifiers such as + or S in \dS are not covered by autocompletion.
>> src/bin/psql/tab-complete.c only specifies backslash commands in their
>> basic form (without modifiers).
>>
>> (\dS actually autocompletes to \ds to my surprise)
>>
>Aha... I never noticed it. Well, with most commands having 1 - 3
>characters it is not a surprised I never used it :)
>That "\dS" autocompletes to "\ds " surprises me even more.
>Thanks for pointing out!

--//--

Good evening, dear all!


Here is the mechanism that implements this:

https://github.com/postgres/postgres/blob/b619852086ed2b5df76631f5678f60d3bebd3745/src/bin/psql/tab-complete.h

.
.
.
1673   /* Match the last N words before point, case-sensitively. */

1674 #define TailMatchesCS(...) \
1675   TailMatchesImpl(true, previous_words_count, previous_words, \

1676   VA_ARGS_NARGS(__VA_ARGS__), __VA_ARGS__)
.
.
.
4824   else if (TailMatchesCS("\\ds*"))
4825 COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_sequences);
.
.
.

There is a rather large list of meta-commands that are handled by 
TailMatchesCS(...).

For example:
\ENCODING autocompletes to \encoding
\eNcOdInG autocompletes to \encoding
\dU or \DU autocompletes to \du

Including the command under discussion:
\CONNINFO autocompletes to \conninfo

For the meta-commands[+], there is no autocomplete.

Regards,
Maiquel Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

>On 2024-02-08 20:37 +0100, Jim Jones wrote:
>> One thing I just noticed. The psql autocomplete feature does not suggest
>> the new + option of \conninfo. For instance, when typing "\connin[TAB]"
>> it automatically autocompletes to "\conninfo ". I guess it should also
>> be included in this patch.
>
>Modifiers such as + or S in \dS are not covered by autocompletion.
>src/bin/psql/tab-complete.c only specifies backslash commands in their
>basic form (without modifiers).
>
>(\dS actually autocompletes to \ds to my surprise)
>
>> I can do a more thorough review of the code when you add the
>> documentation and tests to the patch.
>
>I noticed that the pattern parameter in listConnectionInformation is
>unused.  exec_command_conninfo scans the pattern but \conninfo should
>not accept any argument.  So the pattern can be removed entirely.

--//--

Hi Erik,


Exactly, in "\conninfo+" the "pattern" argument was not introduced and
therefore "listConnectionInformation()" can be declared (signed) without
any arguments. In the future, if necessary, and if there is a need to add
any "pattern", then that can be done. However, that's not the current
scenario. I removed everything related to the "pattern" from the patch.

Regarding "(\dS actually autocompletes to \ds to my surprise)",
I was also surprised. I haven't studied the code yet to understand why
this happens. It seems curious to me. I'll try to understand this
implementation better.

I'm continuing the development of "\conninfo+" and now moving on to tests.

Tks a lot!

Regards,
Maiquel Grassi.


v10-0001-psql-meta-command-conninfo-plus.patch
Description: v10-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

> v8 no longer throws a permission denied error for non-superusers - it is
> IMHO much nicer this way.
>
> $ /usr/local/postgres-dev/bin/psql postgres -p 5432 -h 127.0.0.1 -U jim
> psql (17devel)
> Type "help" for help.
>
> postgres=# \x
> Expanded display is on.
> postgres=# \conninfo+
> Current Connection Information
> -[ RECORD 1 ]--+--
> Database   | postgres
> Authenticated User | jim
> System User|
> Current User   | jim
> Session User   | jim
> Session PID| 1321493
> Server Version | 17devel
> Server Address | 127.0.0.1
> Server Port| 5432
> Client Address | 127.0.0.1
> Client Port| 49366
> Socket Directory   |
> Host   | 127.0.0.1
>
> postgres=# SET ROLE foo;
> SET
> postgres=> \conninfo+
> Current Connection Information
> -[ RECORD 1 ]--+--
> Database   | postgres
> Authenticated User | jim
> System User|
> Current User   | foo
> Session User   | jim
> Session PID| 1321493
> Server Version | 17devel
> Server Address | 127.0.0.1
> Server Port| 5432
> Client Address | 127.0.0.1
> Client Port| 49366
> Socket Directory   |
> Host   | 127.0.0.1
>
> The patch now applies cleanly.
>
> One thing I just noticed. The psql autocomplete feature does not suggest
> the new + option of \conninfo. For instance, when typing "\connin[TAB]"
> it automatically autocompletes to "\conninfo ". I guess it should also
> be included in this patch.
>
> I can do a more thorough review of the code when you add the
> documentation and tests to the patch.
>
> Thanks!

--//--

Hi Jim,


It's not a psql standard to use tab-complete for commands ending with +.
You can verify this in the /* psql's backslash commands. */ section of
the file "/src/bin/psql/tab-complete.c".

https://github.com/postgres/postgres/blob/fdfb92c0307c95eba10854196628d88e6708901e/src/bin/psql/tab-complete.c

In v9, I started the documentation work and am open to suggestions.


> "I can do a more thorough review of the code when you add the

> documentation and tests to the patch."

Soon I'll be developing the tests. And that will be welcome.

Thanks a lot!

Regards,
Maiquel Grassi.


v9-0001-psql-meta-command-conninfo-plus.patch
Description: v9-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

> On 07.02.24 21:13, Maiquel Grassi wrote:
>>
>> I believe in v7 patch we have a quite substantial meta-command feature.
>>
>>
> Thanks for implementing this. I find it very handy.
>
> I was just wondering if a "permission denied" error for non-superusers
> is the best choice for "\conninfo+":
>
> postgres=> \conninfo+
> ERROR:  permission denied to examine "unix_socket_directories"
> DETAIL:  Only roles with privileges of the "pg_read_all_settings" role
> may examine this parameter.
>
> .. since it is not the case with "\conninfo":
>
> postgres=> \conninfo
> You are connected to database "postgres" as user "jim" via socket in
> "/tmp" at port "5432".
>
> Perhaps excluding the column from the result set or returning NULL in
> the affected columns would be less confusing?
>
> There are also some indentation issues in your patch:
>
> /home/jim/Downloads/v7-0001-psql-meta-command-conninfo-plus.patch:142:
> indent with spaces.
> PGresult   *res;
> /home/jim/Downloads/v7-0001-psql-meta-command-conninfo-plus.patch:143:
> indent with spaces.
> PQExpBufferData buf;
> /home/jim/Downloads/v7-0001-psql-meta-command-conninfo-plus.patch:144:
> indent with spaces.
> printQueryOpt myopt = pset.popt;
> /home/jim/Downloads/v7-0001-psql-meta-command-conninfo-plus.patch:146:
> indent with spaces.
> initPQExpBuffer();
> /home/jim/Downloads/v7-0001-psql-meta-command-conninfo-plus.patch:148:
> indent with spaces.
> printfPQExpBuffer(,
> warning: squelched 34 whitespace errors
> warning: 39 lines add whitespace errors.
>
> Looking forward to see the documentation and tests!

--//--

Hi Jim,
Thank you for your support on this patch!
As I believe in its usability, I have been dedicating efforts to make it really 
interesting.
I hadn't thought about the permissioning issue for "unix_socket_directories". I 
appreciate that.
I thought about solving this situation using the same approach as \conninfo. I 
added the validation if (is_unixsock_path(host) && !(hostaddr && *hostaddr)) in 
the SQL part along with an "append". In case of a negative result, another 
"append" adds NULL.
Regarding the whitespace issue, before generating v8 patch file, I used 
pgindent to adjust each modified file. I believe it should be ok now. If you 
could verify, I'd be grateful.
Below are the tests after adjusting for the permissioning issues:

[postgres@localhost bin]$ ./psql
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+

  Current Connection Information
 Database | Authenticated User | System User | Current User | Session User | 
Session PID | Server Version | Server Address | Server Port | Client Address | 
Client Port | Socket Directory | Host
--++-+--+--+-+++-++-+--+--
 postgres | postgres   | | postgres | postgres |
   31479 | 17devel|| 5432|| 
| /tmp |
(1 row)

postgres=# CREATE USER maiquel;
CREATE ROLE
postgres=# \q
[postgres@localhost bin]$ ./psql -U maiquel -d postgres
psql (17devel)
Type "help" for help.

postgres=> \conninfo
You are connected to database "postgres" as user "maiquel" via socket in "/tmp" 
at port "5432".
postgres=> \conninfo+

  Current Connection Information
 Database | Authenticated User | System User | Current User | Session User | 
Session PID | Server Version | Server Address | Server Port | Client Address | 
Client Port | Socket Directory | Host
--++-+--+--+-+++-++-+--+--
 postgres | maiquel| | maiquel  | maiquel  |
   31482 | 17devel|| 5432|| 
| /tmp |
(1 row)

postgres=> \q
[postgres@localhost bin]$ ./psql -h localhost -U maiquel -d postgres
psql (17devel)
Type "help" for help.

postgres=> \conninfo
You are connected to database "

RE: Psql meta-command conninfo+

[Maiquel Grassi]

> Hi,

> On 07.02.2024 23:13, Maiquel Grassi wrote:


> Regarding the "system_user" function, as it is relatively new, I added
> the necessary handling to avoid conflicts with versions lower than version 16.


> Yes, that's rights.
>
> A couple of doubts about the implementation details.
> But keep in mind that I'm not very good at programming in the C language.
> I hope for the help of more experienced developers.
>
>
> 1.
> + if (db == NULL)
> + printf(_("You are currently not connected to a 
> database.\n"));
>
> This check is performed for \conninfo, but not for \conninfo+.
>
>
> 2.
> Some values (address, socket) are evaluated separately for \conninfo
> (via C functions) and for \conninfo+ (via sql functions).
> It may be worth evaluating them in one place. But I'm not sure about that.
>
> The final version of the patch will require changes to the documentation and 
> tests.

--//--

Hi Pavel,

First of all, thank you very much for your observations.

1. The connection check for the case of \conninfo+ is handled by "describe.c" 
itself since it deals with queries. I might be mistaken, but I believe that by 
using "printQuery()" via "describe.c", this is already ensured, and there is no 
need to evaluate the connection status.

2. I believe that by implementing the evaluations separately as they are, it 
becomes easier to perform future maintenance by minimizing the mixing of C code 
with SQL code as much as possible. However, certainly, the possibility of a 
better suggestion than mine remains open.

Regards,
Maiquel O. Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

> Hi, Maiquel!
>
> On 07.02.2024 17:47, Maiquel Grassi wrote:

> "Also, it seems that the verbose parameter in the listConnectionInformation 
> is unnecessary."
> Could you point out exactly the line or code snippet you are referring to?

> +bool
> +listConnectionInformation(const char *pattern, bool verbose)
>
> I mean that parameter verbose is not used in the function body and 
> listConnectionInformation called only in verbose mode.


--//--

There really was no need for the bool verbose. Therefore, it was removed.
Regarding the "system_user" function, as it is relatively new, I added
the necessary handling to avoid conflicts with versions lower than version 16.

I believe in v7 patch we have a quite substantial meta-command feature.

I validated the "System User" column for three versions. This way, we have a 
sample:

[postgres@localhost bin]$ ./psql -h 192.168.0.5 -p 5432 -U postgres -d postgres

psql (17devel, server 14.3)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host 
"192.168.0.5" at port "5432".
postgres=# \conninfo+
  
Current Connection Information
 Database | Authenticated User | Current User | Session User | Session PID | 
Server Version | Server Address | Server Port | Client Address | Client Port | 
Socket Directory |Host
--++--+--+-+++-++-+--+-
 postgres | postgres   | postgres | postgres |9008 | 
14.3   | 192.168.0.5| 5432| 192.168.0.5|   63631 |  
| 192.168.0.5
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h 192.168.0.5 -p 5433 -U postgres -d postgres
psql (17devel, server 16.1)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host 
"192.168.0.5" at port "5433".
postgres=# \conninfo+

 Current Connection Information
 Database | Authenticated User | System User | Current User | Session User | 
Session PID | Server Version | Server Address | Server Port | Client Address | 
Client Port | Socket Directory |Host
--++-+--+--+-+++-++-+--+-
 postgres | postgres   | | postgres | postgres |
3348 | 16.1   | 192.168.0.5| 5433| 192.168.0.5| 
  63633 |  | 192.168.0.5
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+

Current Connection Information
 Database | Authenticated User | System User | Current User | Session User | 
Session PID | Server Version | Server Address | Server Port | Client Address | 
Client Port | Socket Directory |   Host
--++-+--+--+-+++-++-+--+---
 postgres | postgres   | | postgres | postgres |
   26147 | 17devel| ::1| 5432| ::1| 
  47466 | /tmp | localhost
(1 row)

Regards,
Maiquel O. Grassi.


v7-0001-psql-meta-command-conninfo-plus.patch
Description: v7-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

  This is a good idea about extended connection info.

  On 07.02.2024 07:13, Maiquel Grassi wrote:
SELECT
...
  current_user AS "User",

  This will be inconsistent with \conninfo.
  \conninfo returns authenticated user (PQuser), not the current_user.
  It might be worth showing current_user, session_user, and authenticated user,
  but I can't find the appropriate sql function for PQuser.

  What about to include system_user function? It shows useful authentication 
details.

  Also, it seems that the verbose parameter in the listConnectionInformation
  is unnecessary.

--//--

Hi,

Tks Pavel.

Analyzing the functions' code more thoroughly, it seems to make more sense.
I liked your suggestions and implemented them for validation.
Regarding "system_user," I believe it is valid and also added it to the row.

"Also, it seems that the verbose parameter in the listConnectionInformation is 
unnecessary."
Could you point out exactly the line or code snippet you are referring to?

To print the string from the "Authenticated User" column, I chose to use 
PQuser(pset.db) directly. I did the same for the "Host" column, opting for 
PQhost(pset.db). This does not contradict the result of \conninfo.

Here are the tests as usual, and v6 patch.

[postgres@localhost bin]$ ./psql
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+

  Current Connection Information
 Database | Authenticated User | Current User | Session User | System User | 
Server Version | Server Address | Server Port | Client Address | Client Port | 
Session PID | Socket Directory | Host
--++--+--+-+++-++-+-+--+--
 postgres | postgres   | postgres | postgres | | 
17devel|| 5432|| |  
 17240 | /tmp |
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h ::1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "::1" at 
port "5432".
postgres=# \conninfo+

  Current Connection Information
 Database | Authenticated User | Current User | Session User | System User | 
Server Version | Server Address | Server Port | Client Address | Client Port | 
Session PID | Socket Directory | Host
--++--+--+-+++-++-+-+--+--
 postgres | postgres   | postgres | postgres | | 
17devel| ::1| 5432| ::1|   47024 |  
 17242 | /tmp | ::1
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h 127.0.0.1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5432".
postgres=# \conninfo+

Current Connection Information
 Database | Authenticated User | Current User | Session User | System User | 
Server Version | Server Address | Server Port | Client Address | Client Port | 
Session PID | Socket Directory |   Host
--++--+--+-+++-++-+-+--+---
 postgres | postgres   | postgres | postgres | | 
17devel| 127.0.0.1  | 5432| 127.0.0.1  |   35076 |  
 17245 | /tmp | 127.0.0.1
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+

Current Connection Information
 Database | Authenticated User | Current User | Session User | System User | 
Server Version | Server Address | Server Port | Client Address | Client Port | 
Session PID | Socket Directory |   Host
--++--+--+-++-

RE: Psql meta-command conninfo+

[Maiquel Grassi]

  On 2024-02-07 05:13 +0100, Maiquel Grassi wrote:
  >   On Tue, Feb 06, 2024 at 09:45:54PM +0000, Maiquel Grassi wrote:
  >   > My initial idea has always been that they should continue to appear
  >   > because \conninfo+ should show all the things that \conninfo shows and
  >   > add more information. I think that's the purpose of the 'plus.' Now 
we're
  >   > on a better path than the initial one. We can still add the socket
  >   > directory and the host.
  >
  >   Agreed.
  >
  > --//--
  >
  > I believe it's resolved reasonably well this way:
  >
  > SELECT
  >   pg_catalog.current_database() AS "Database",
  >   current_user AS "User",
  >   pg_catalog.current_setting('server_version') AS "Server Version",
  >   CASE
  > WHEN pg_catalog.inet_server_addr() IS NULL
  > THEN 'NULL'
  > ELSE pg_catalog.inet_server_addr()::text
  >   END AS "Server Address",

  Should be NULL instead of string 'NULL'.  So the entire CASE expression
  is redundant and you can just return pg_catalog.inet_server_addr().

  >   pg_catalog.current_setting('port') AS "Port",
  >   CASE
  > WHEN pg_catalog.inet_client_addr() IS NULL
  > THEN 'NULL'
  > ELSE pg_catalog.inet_client_addr()::text
  >   END AS "Client Address",
  >   CASE
  > WHEN pg_catalog.inet_client_port() IS NULL
  >  THEN 'NULL'
  > ELSE pg_catalog.inet_client_port()::text
  >   END AS "Client Port",

  Same here.

  >   pg_catalog.pg_backend_pid() AS "Session PID",
  >   CASE
  > WHEN pg_catalog.current_setting('unix_socket_directories') = ''
  > THEN 'NULL'
  > ELSE pg_catalog.current_setting('unix_socket_directories')
  >   END AS "Socket Directory",

  The CASE expression can be simplified to:

  nullif(pg_catalog.current_setting('unix_socket_directories'), '')

  >   CASE
  > WHEN
  >   pg_catalog.inet_server_addr() IS NULL
  >   AND pg_catalog.inet_client_addr() IS NULL
  > THEN 'NULL'
  > WHEN
  >   pg_catalog.inet_server_addr() = pg_catalog.inet_client_addr()
  > THEN 'localhost'

  Is it safe to assume localhost here?  \conninfo prints localhost only
  when I connect with psql -hlocalhost:

  $ psql -hlocalhost postgres
  psql (16.1)
  postgres=# \conninfo
  You are connected to database "postgres" as user "ewie" on host 
"localhost" (address "::1") at port "5432".
  postgres=# \q

  $ psql -h127.0.0.1 postgres
  psql (16.1)
  postgres=# \conninfo
  You are connected to database "postgres" as user "ewie" on host 
"127.0.0.1" at port "5432".

  > ELSE pg_catalog.inet_server_addr()::text
  >   END AS "Host";

--//--

There really was no need for the CASES. However, they helped visualize the psql 
output since for the null value, no word is printed on the screen. I made the 
adjustment by removing this redundancy.

Regarding the "Host" column, the most reliable way to solve this, I believe, is 
by using the "host" variable. So it's necessary to declare char *host = 
PQhost(pset.db); in listConnectionInformation() and use it in the SQL (see 
patch v5). This way, we have the same return from \conninfo reliably.

Once again, I ran a series of tests.

[postgres@localhost bin]$ ./psql
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
   Current Connection 
Information
 Database |   User   | Server Version | Server Address | Server Port | Client 
Address | Client Port | Session PID | Socket Directory | Host
--+--+++-++-+-+--+--
 postgres | postgres | 17devel|| 5432|  
  | |   15898 | /tmp |
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+
 Current Connection 
Information
 Database |   User   | Server Version | Server Address | Server Port | Client 
Address | Client Port | Session PID | Socket Directory |   Host
--+--++

RE: Psql meta-command conninfo+

[Maiquel Grassi]

  On Tue, Feb 06, 2024 at 09:45:54PM +, Maiquel Grassi wrote:
  > My initial idea has always been that they should continue to appear
  > because \conninfo+ should show all the things that \conninfo shows and
  > add more information. I think that's the purpose of the 'plus.' Now we're
  > on a better path than the initial one. We can still add the socket
  > directory and the host.

  Agreed.

--//--

I believe it's resolved reasonably well this way:

SELECT
  pg_catalog.current_database() AS "Database",
  current_user AS "User",
  pg_catalog.current_setting('server_version') AS "Server Version",
  CASE
WHEN pg_catalog.inet_server_addr() IS NULL
THEN 'NULL'
ELSE pg_catalog.inet_server_addr()::text
  END AS "Server Address",
  pg_catalog.current_setting('port') AS "Port",
  CASE
WHEN pg_catalog.inet_client_addr() IS NULL
THEN 'NULL'
ELSE pg_catalog.inet_client_addr()::text
  END AS "Client Address",
  CASE
WHEN pg_catalog.inet_client_port() IS NULL
THEN 'NULL'
ELSE pg_catalog.inet_client_port()::text
  END AS "Client Port",
  pg_catalog.pg_backend_pid() AS "Session PID",
  CASE
WHEN pg_catalog.current_setting('unix_socket_directories') = ''
THEN 'NULL'
ELSE pg_catalog.current_setting('unix_socket_directories')
  END AS "Socket Directory",
  CASE
WHEN
  pg_catalog.inet_server_addr() IS NULL
  AND pg_catalog.inet_client_addr() IS NULL
THEN 'NULL'
WHEN
  pg_catalog.inet_server_addr() = pg_catalog.inet_client_addr()
THEN 'localhost'
ELSE pg_catalog.inet_server_addr()::text
  END AS "Host";

See below the tests:

[postgres@localhost bin]$ ./psql
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
   Current Connection 
Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID | Socket Directory | Host
--+--+++--++-+-+--+--
 postgres | postgres | 17devel| NULL   | 5432 | NULL   
| NULL|   14348 | /tmp | NULL
(1 row)


[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+
  Current Connection 
Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID | Socket Directory |   Host
--+--+++--++-+-+--+---
 postgres | postgres | 17devel| ::1/128| 5432 | ::1/128
| 46988   |   14353 | /tmp | localhost
(1 row)


[postgres@localhost bin]$ ./psql -h ::1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "::1" at 
port "5432".
postgres=# \conninfo+
  Current Connection 
Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID | Socket Directory |   Host
--+--+++--++-+-+--+---
 postgres | postgres | 17devel| ::1/128| 5432 | ::1/128
| 46990   |   14356 | /tmp | localhost
(1 row)


[postgres@localhost bin]$ ./psql -h 127.0.0.1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5432".
postgres=# \conninfo+
  Current Connection 
Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID | Socket Directory |   Host
--+--+++--++-+-+--+---
 postgres | postgres | 17devel| 127.0.0.1/32   | 5432 | 127.0.0.1/32   
| 35042   |   14359 | /tmp | localhost
(1 row)


[postgres@localhost bin]$ ./psql -h 192.168.0.5 -p 5433 -U postgres -d postgres
psql (17devel, server 16.1)
Type "help" for help.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

  On Tue, Feb 06, 2024 at 03:06:05PM -0600, Nathan Bossart wrote:
  > On Tue, Feb 06, 2024 at 08:52:09PM +0000, Maiquel Grassi wrote:
  >> I made the adjustment in the code and updated the patch. I believe this
  >> is the format suggested by you all. Would this be it?
  >
  > I was thinking something more like
  >
  >   SELECT pg_catalog.current_database() AS "Database",
  >  current_user AS "User",
  >  pg_catalog.current_setting('server_version') AS "Server Version",
  >  pg_catalog.inet_server_addr() AS "Server Address",
  >  pg_catalog.current_setting('port') AS "Port",
  >  pg_catalog.inet_client_addr() AS "Client Address",
  >  pg_catalog.inet_client_port() AS "Client Port",
  >  pg_catalog.pg_backend_pid() AS "Session PID";

  ... although that seems to be missing items like the socket directory and
  the host.

--//--

My initial idea has always been that they should continue to appear because 
\conninfo+ should show all the things that \conninfo shows and add more 
information. I think that's the purpose of the 'plus.' Now we're on a better 
path than the initial one. We can still add the socket directory and the host.

Regards,
Maiquel O. Grassi.

RE: Psql meta-command conninfo+

[Maiquel Grassi]

  On Tue, Feb 06, 2024 at 08:52:09PM +, Maiquel Grassi wrote:
  > I made the adjustment in the code and updated the patch. I believe this
  > is the format suggested by you all. Would this be it?

  I was thinking something more like

SELECT pg_catalog.current_database() AS "Database",
   current_user AS "User",
   pg_catalog.current_setting('server_version') AS "Server Version",
   pg_catalog.inet_server_addr() AS "Server Address",
   pg_catalog.current_setting('port') AS "Port",
   pg_catalog.inet_client_addr() AS "Client Address",
   pg_catalog.inet_client_port() AS "Client Port",
   pg_catalog.pg_backend_pid() AS "Session PID";

--//--

Good, I had misunderstood. I liked this adjustment. Now it truly aligns with 
the central idea of the other extended meta-commands.

[postgres@localhost bin]$ ./psql -h 192.168.0.220 -p 5433 -U postgres -d 
postgres
psql (17devel, server 16.1)
Type "help" for help.

postgres=# \conninfo+
  Current Connection Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID
--+--+++--++-+-
 postgres | postgres | 16.1   | 192.168.0.220  | 5433 | 192.168.0.220  
|   57112 |   22120
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql
psql (17devel)
Type "help" for help.

postgres=# \conninfo+
  Current Connection Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID
--+--+++--++-+-
 postgres | postgres | 17devel|| 5432 |
| |   31430
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo+
  Current Connection Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID
--+--+++--++-+-
 postgres | postgres | 17devel| ::1| 5432 | ::1
|   46918 |   31433
(1 row)

postgres=# \q
[postgres@localhost bin]$ ./psql -h 127.0.0.1
psql (17devel)
Type "help" for help.

postgres=# \conninfo+
  Current Connection Information
 Database |   User   | Server Version | Server Address | Port | Client Address 
| Client Port | Session PID
--+--+++--++-+-
 postgres | postgres | 17devel| 127.0.0.1  | 5432 | 127.0.0.1  
|   34970 |   31435
(1 row)

Regards,
Maiquel O. Grassi.


v3-0001-psql-meta-command-conninfo-plus.patch
Description: v3-0001-psql-meta-command-conninfo-plus.patch

RE: Psql meta-command conninfo+

[Maiquel Grassi]

  On 2024-02-06 19:19 +0100, Nathan Bossart wrote:
  > On Tue, Feb 06, 2024 at 05:27:01PM +0000, Maiquel Grassi wrote:
  > > postgres=# \conninfo+
  > >  Current Connection Information
  > >Attribute| Value
  > > +
  > >  Database   | postgres
  > >  User   | postgres
  > >  Server Version | 16.1
  > >  Server Address | 192.168.0.5/32
  > >  Server Port| 5433
  > >  Client Address | 192.168.0.5/32
  > >  Client Port| 52716
  > >  Session PID| 21624
  > > (8 rows)
  >
  > My first reaction is that this should instead return a single row with the
  > same set of columns for any connection type (the not-applicable ones would
  > just be set to NULL).  That would match the other meta-commands like \l and
  > \du, and you could still get an expanded display with \x if needed.  Also,
  > I think it would simplify the code a bit.

  +1 for a single-row result and triggering expanded display with \x for
  consistency with other commands.

--//--

I made the adjustment in the code and updated the patch. I believe this is the 
format suggested by you all. Would this be it?

[postgres@localhost bin]$ ./psql -h 192.168.0.220 -p 5433 -U postgres -d 
postgres
psql (17devel, server 16.1)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host 
"192.168.0.220" at port "5433".
postgres=# \conninfo+
  Current Connection Information
   Attribute|  Value
+--
 Database   | postgres
 User   | postgres
 Server Version | 16.1
 Server Address | 192.168.0.220/32
 Server Port| 5433
 Client Address | 192.168.0.220/32
 Client Port| 56606
 Session PID| 2424
(8 rows)

[postgres@localhost bin]$ ./psql
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
Current Connection Information
   Attribute|  Value
+--
 Database   | postgres
 User   | postgres
 Server Version | 17devel
 Server Address |
 Server Port| 5432
 Client Address |
 Client Port|
 Session PID| 30216
(8 rows)

[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+
Current Connection Information
   Attribute|  Value
+--
 Database   | postgres
 User   | postgres
 Server Version | 17devel
 Server Address | ::1/128
 Server Port| 5432
 Client Address | ::1/128
 Client Port| 46872
 Session PID| 30220
(8 rows)

[postgres@localhost bin]$ ./psql -h 127.0.0.1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5432".
postgres=# \conninfo+
Current Connection Information
   Attribute|Value
+--
 Database   | postgres
 User   | postgres
 Server Version | 17devel
 Server Address | 127.0.0.1/32
 Server Port| 5432
 Client Address | 127.0.0.1/32
 Client Port| 34924
 Session PID| 30223
(8 rows)

Regards,
Maiquel O. Grassi.


v2-0001-psql-meta-command-conninfo-plus.patch
Description: v2-0001-psql-meta-command-conninfo-plus.patch

Psql meta-command conninfo+

[Maiquel Grassi]

Hi,


I'm seeking to improve the \conninfo meta-command in psql. Currently, it 
provides limited information about the current connection. I believe that 
expanding it using the concept of "plus" [+] could ease the work of DBAs, 
SysAdmins, DevOps, etc., who manage a large volume of databases and/or multiple 
PostgreSQL servers. The objective of this enhancement is to obtain quick 
information about the current connection (session). I believe that for a 
PostgreSQL administrator, it is not feasible to write a plpgsql function and 
apply it to all databases (for example, imagine managing over 200 databases). I 
have an example on GitHub 
https://github.com/maiquelgrassi/DBA-toolkit/blob/main/cluster/dba_whoami_function.sql
 of a plpgsql function demonstrating exactly what I believe is impractical for 
the daily routine of a PostgreSQL professional. I see psql's meta-commands as 
significant allies in daily work in productive environments.


Note: As this is a prototype, I will adjust the rest (documentation, tests, 
etc.) once an agreement is reached.

Use cases for both the current and improved command bellow.

Connection 1 ("remote server"):

[postgres@localhost bin]$ ./psql -h 192.168.0.5 -p 5433 -U postgres -d postgres

psql (17devel, server 16.1)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host 
"192.168.0.5" at port "5433".
postgres=# \conninfo+
 Current Connection Information
   Attribute| Value
+
 Database   | postgres
 User   | postgres
 Server Version | 16.1
 Server Address | 192.168.0.5/32
 Server Port| 5433
 Client Address | 192.168.0.5/32
 Client Port| 52716
 Session PID| 21624
(8 rows)


Connection 2 (socket):

[postgres@localhost bin]$ ./psql

psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" via socket in 
"/tmp" at port "5432".
postgres=# \conninfo+
  Current Connection Information
Attribute | Value
--+---
 Info | Connected via socket!
 Database | postgres
 User | postgres
 Socket Directory | /tmp
 Server Version   | 17devel
 Server Port  | 5432
 Session PID  | 27586
(7 rows)

Connection 3 (localhost):
[postgres@localhost bin]$ ./psql -h localhost
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "localhost" 
(address "::1") at port "5432".
postgres=# \conninfo+
Current Connection Information
   Attribute|   Value
+---
 Database   | postgres
 User   | postgres
 Host   | localhost
 Server Version | 17devel
 Server Address | ::1/128
 Server Port| 5432
 Client Address | ::1/128
 Client Port| 46824
 Session PID| 27598
(9 rows)

Connection 4 (127.0.0.1):
[postgres@localhost bin]$ ./psql -h 127.0.0.1
psql (17devel)
Type "help" for help.

postgres=# \conninfo
You are connected to database "postgres" as user "postgres" on host "127.0.0.1" 
at port "5432".
postgres=# \conninfo+
Current Connection Information
   Attribute|Value
+--
 Database   | postgres
 User   | postgres
 Server Version | 17devel
 Server Address | 127.0.0.1/32
 Server Port| 5432
 Client Address | 127.0.0.1/32
 Client Port| 34876
 Session PID| 27624
(8 rows)

Regards,
Maiquel O. Grassi.


v1-0001-psql-meta-command-conninfo-plus.patch
Description: v1-0001-psql-meta-command-conninfo-plus.patch

RE: Current Connection Information

[Maiquel Grassi]

Hi Aleksander,

>>I assume you wanted to reply to the mailing list and add me to cc:
>>(aka "Reply to All") but sent the e-mail off-list by mistake, so
>>quoting it here:

Yes, tks for that.

>>IMO it's worth trying submitting the patch, if your time permits it of course.

I've been spending a little time thinking about this.

Regards,
Maiquel.

Current Connection Information

[Maiquel Grassi]

Hi,

It would be viable and appropriate to implement a unified function that 
provides important information about the current connection?

Just an example: "Current Connection Informations".

I implemented it in PL/pgSQL to demonstrate the idea, see on GitHub:
https://github.com/maiquelgrassi/DBA-toolkit/blob/main/cluster/dba_whoami_function.sql

Regards,
Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

Even if a workable patch for that is presented, should we accept it?
I'm having a hard time believing that this requirement is common
enough to justify more than a microscopic addition of complexity.
This whole area is devilishly complicated already, and I can think of
a bunch of improvements that I'd rate as more worthy of developer
effort than this.

--//--


Thanks for the advice. I understand that an improvement you consider 
microscopic may not be worth spending time trying to implement it (considering 
you are already warning that a good patch might not be accepted). But since you 
mentioned that you can think of several possible improvements, more worthy of 
time investment, could you share at least one of them with us that you consider 
a candidate for an effort?

Regards,
Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

As far as I see your proposal, you want to allow something that is
undefined to be reversed.  I don't think this is a good idea at all.
As mentioned by others, you should have ORDER BY clauses and just add
a DESC.


  *   Okay, now I'm convinced of that.

If you were looking for something to optimize in this rough area, then
perhaps adding some kind of "Backward WindowAgg" node (by overloading
the existing node) to allow queries such as the following to be
executed without an additional sort.

SELECT a,row_number() over (order by a desc) from t order by a;


  *   David, considering this optimization, allowing for that, do you believe 
it is plausible to try advancing towards a possible Proof of Concept (PoC) 
implementation?

Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

As far as I see your proposal, you want to allow something that is
undefined to be reversed.  I don't think this is a good idea at all.
As mentioned by others, you should have ORDER BY clauses and just add
a DESC.

If you were looking for something to optimize in this rough area, then
perhaps adding some kind of "Backward WindowAgg" node (by overloading
the existing node) to allow queries such as the following to be
executed without an additional sort.

SELECT a,row_number() over (order by a desc) from t order by a;

The planner complexity is likely fairly easy to implement that. I
don't think we'd need to generate any additional Paths. We could
invent some pathkeys_contained_in_reverse() function and switch on the
Backward flag if it is.

The complexity would be in nodeWindowAgg.c... perhaps too much
complexity for it to be worthwhile and not add additional overhead to
the non-backward case.

Or, it might be easier to invent "Backward Materialize" instead and
just have the planner use on of those instead of the final sort.

David

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

But as you are the one arguing for the new feature demonstrating that the 
status quo is deficient is your job.

--//--

I performed these three tests(take a look below) quite simple but functional, 
so that we can get an idea of the performance. Apparently, we have a higher 
cost in using "count(*) - row_number() + 1" than in using "row_number_desc() 
over()".

Perhaps, if we think in terms of SQL standards, my suggested name may not have 
been the best. The name could be anything else. I don't have another 
suggestion. Does anyone have a better one? I leave it open for others to also 
reflect.



postgres=# select * into public.foo_1 from generate_series(1,100);
SELECT 100
postgres=# explain analyze select count(*) over() - row_number() over() + 1 
from public.foo_1;
  QUERY PLAN
--
 WindowAgg  (cost=0.00..38276.25 rows=1128375 width=8) (actual 
time=244.878..475.595 rows=100 loops=1)
   ->  Seq Scan on foo_1  (cost=0.00..15708.75 rows=1128375 width=0) (actual 
time=0.033..91.486 rows=100 loops=1)
 Planning Time: 0.073 ms
 Execution Time: 505.375 ms
(4 rows)

postgres=# explain analyze select row_number_desc() over() from public.foo_1;
  QUERY PLAN
--
 WindowAgg  (cost=0.00..26925.00 rows=100 width=8) (actual 
time=141.107..427.100 rows=100 loops=1)
   ->  Seq Scan on foo_1  (cost=0.00..14425.00 rows=100 width=0) (actual 
time=0.031..61.651 rows=100 loops=1)
 Planning Time: 0.051 ms
 Execution Time: 466.535 ms
(4 rows)



postgres=# select * into public.foo_2 from generate_series(1,1000);
SELECT 1000
postgres=# explain analyze select count(*) over() - row_number() over() + 1 
from public.foo_2;
   QUERY PLAN
-
 WindowAgg  (cost=0.00..344247.31 rows=977 width=8) (actual 
time=2621.014..5145.325 rows=1000 loops=1)
   ->  Seq Scan on foo_2  (cost=0.00..144247.77 rows=977 width=0) (actual 
time=0.031..821.533 rows=1000 loops=1)
 Planning Time: 0.085 ms
 Execution Time: 5473.422 ms
(4 rows)

postgres=# explain analyze select row_number_desc() over() from public.foo_2;
   QUERY PLAN
-
 WindowAgg  (cost=0.00..269247.48 rows=977 width=8) (actual 
time=1941.915..4527.896 rows=1000 loops=1)
   ->  Seq Scan on foo_2  (cost=0.00..144247.77 rows=977 width=0) (actual 
time=0.029..876.802 rows=1000 loops=1)
 Planning Time: 0.030 ms
 Execution Time: 4871.278 ms
(4 rows)




postgres=# select * into public.foo_3 from generate_series(1,1);
SELECT 1
postgres=# explain analyze select count(*) over() - row_number() over() + 1 
from public.foo_3;
  QUERY PLAN
---
 WindowAgg  (cost=0.00..3827434.70 rows=112831890 width=8) (actual 
time=56823.080..84295.660 rows=1 loops=1)
   ->  Seq Scan on foo_3  (cost=0.00..1570796.90 rows=112831890 width=0) 
(actual time=1.010..37735.121 rows=1 loops=1)
 Planning Time: 1.018 ms
 Execution Time: 87677.572 ms
(4 rows)

postgres=# explain analyze select row_number_desc() over() from public.foo_3;
   QUERY PLAN

 WindowAgg  (cost=0.00..2981195.53 rows=112831890 width=8) (actual 
time=29523.037..55517.349 rows=1 loops=1)
   ->  Seq Scan on foo_3  (cost=0.00..1570796.90 rows=112831890 width=0) 
(actual time=12.638..19050.614 rows=1 loops=1)
 Planning Time: 55.653 ms
 Execution Time: 59001.423 ms
(4 rows)



Regards,
Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

The people in this community are quite capable and willing to write a contrary 
opinion to mine.  Not sure how to make “this new proposed function shouldn’t be 
added to core”, and trying to explain why not, non-oppressive.  I can add 
“thank you for taking the time to try and improve PostgreSQL” in front to 
soften the blow of rejection but I tend to just get to the point.

David J.

//

Thank you for your opinion. We built together one more insight on PostgreSQL 
for the community.

Best regards,
Maiquel O.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

However, initially, I have one more obstacle in your feedback. If I use 
count(*) over() - row_number() over(), it gives me an offset of one unit. To 
resolve this, I need to add 1.

This way, simulating a reverse row_number() becomes even more laborious.

I don’t really understand why you think this reverse inserted counting is even 
a good idea so I don’t really care how laborious it is to implement with 
existing off-the-shelf tools.  A window function named “descending” is 
non-standard and seemingly non-sensical and should not be added.  You can 
specify order by in the over clause and that is what you should be doing.  
Mortgage payments are usually monthly, so order by date.

David J.

--//--

We are just raising hypotheses and discussing healthy possibilities here. This 
is a suggestion for knowledge and community growth. Note that this is not about 
a new "feature patch." I am asking for the community's opinion in general. Your 
responses are largely appearing aggressive and depreciative. Kindly request you 
to be more welcoming in your answers and not oppressive. This way, the 
community progresses more rapidly.

Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

You can do:

-(ROW_NUMBER() OVER ()) AS descending

(note “-“ in front)

I don't have a base column to use for "order by,"

I think that’s the main issue: what (semantically) does row_number() mean in 
that case? You could equally well generate random numbers?


--//--

What I want to do is inverse the enumeration using a simple solution. I want to 
look at the enumeration of the dataset list from bottom to top, not from top to 
bottom. I don't want to reverse the sign of the integers. The generated 
integers in output remain positive.The returned dataset can be from any query. 
What I need is exactly the opposite of row_number().

count(*) over() - row_number() + 1 works.

But I think for a large volume of data, its performance will be inferior to the 
suggested row_number_desc() over(). I may be very wrong, so I will test it.

Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

I doubt it is materially different, you need that count regardless so the 
effort is expended no matter if you put it in an SQL expression or build it 
into the window function.  But as you are the one arguing for the new feature 
demonstrating that the status quo is deficient is your job.

---//---

Ok, I'll run the tests to validate these performances and draw some conclusions.

However, initially, I have one more obstacle in your feedback. If I use 
count(*) over() - row_number() over(), it gives me an offset of one unit. To 
resolve this, I need to add 1. This way, simulating a reverse row_number() 
becomes even more laborious.

SELECT
  row_number() over()
  , row_number_desc() over()
  , count(*) over() - row_number() over() as FROM pg_catalog.pg_database;
 row_number | row_number_desc | count_minus_row_number
+-+
  1 |   3 |  2
  2 |   2 |  1
  3 |   1 |  0
(3 rows)

postgres=# SELECT row_number() over(), row_number_desc() over(), count(*) 
over() - row_number() over() as count_minus_row_number, count(*) over() - 
row_number() over() + 1 AS count_minus_row_number_plus_one FROM 
pg_catalog.pg_database;
 row_number | row_number_desc | count_minus_row_number | 
count_minus_row_number_plus_one
+-++-
  1 |   3 |  2 |
   3
  2 |   2 |  1 |
   2
  3 |   1 |  0 |
   1
(3 rows)

Tks,
Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

Hi,

Count() over() - row_number() over()

   But if my dataset is significantly large? Wouldn't calling two window 
functions instead of one be much slower?
   Is count() over() - row_number() over() faster than row_number_desc() over()?

Maiquel.

RE: New Window Function: ROW_NUMBER_DESC() OVER() ?

[Maiquel Grassi]

Hello David, how are you?

Firstly, I apologize if I wasn't clear in what I intended to propose. I used a 
very specific example here, and it wasn't very clear what I really wanted to 
bring up for discussion.

I understand that it's possible to order the "returned dataset" using "order by 
... desc." However, I would like someone to help me think about the following 
scenario:

Imagine I have a dataset that is returned to my front-end, and I want to 
reverse enumerate them (exactly the concept of Math enumerating integers). The 
row_number does the ascending enumeration, but I need the descending 
enumeration. I don't have a base column to use for "order by," and I also can't 
use CTID column.

Furthermore, imagine that I have a list of hashes, and I would use "order by" 
on this column or another column to do the reverse enumeration. This wouldn't 
work because I wouldn't have the correct reverse enumeration, meaning the 
reversal of the data would not be original.

It's not about reverse ordering; it's about reverse enumeration.

I apologize again for not being clear in the first interaction.

How can I do this without using my reversed enumeration "row_number desc" 
function?

Regards,
Maiquel O. Grassi.

De: David G. Johnston 
Enviado: terça-feira, 16 de janeiro de 2024 11:30
Para: Maiquel Grassi 
Cc: pgsql-hack...@postgresql.org 
Assunto: Re: New Window Function: ROW_NUMBER_DESC() OVER() ?

On Tuesday, January 16, 2024, Maiquel Grassi 
mailto:gra...@hotmail.com.br>> wrote:
Hi developers,

I was working on loans and bank financing, specifically focusing on 
Amortization Systems. I had the need to reverse the counter for the total 
number of installments or for a specific set of installments. This "reversal" 
is essentially a reverse "row_number" function. I realized that it is to "hard 
work" to write PL/foo functions for this or even to implement it in just SQL 
using little code.

I think “row_number() over (order by … desc)”  is a sufficient way to get this 
behavior and this isn’t something useful enough to warrant being the first 
ordering-specific function in the system.

David J.