Message-ID: <cefe290e-4e70-4cfd-89e9-888e916f6b51@uni-muenster.de>
Date: Thu, 8 Feb 2024 20:37:38 +0100
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Psql meta-command conninfo+
Content-Language: en-US
To: Maiquel Grassi <grassi@hotmail.com.br>,
 Pavel Luzanov <p.luzanov@postgrespro.ru>,
 Nathan Bossart <nathandbossart@gmail.com>
Cc: Erik Wienhold <ewie@ewie.name>,
 "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: <CP8P284MB24965CB63DAC00FC0EA4A475EC462@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
 <20240206181919.GA3853632@nathanxps13>
 <aevq2dxmw2xm2rc3nh2wdcozunipco447mf4rpm7efvqvz4wk2@mo3zmf2ulbzu>
 <CP8P284MB2496B51FC5ED9BD6527D459DEC462@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
 <20240206210605.GA3903769@nathanxps13> <20240206211205.GA3903996@nathanxps13>
 <CP8P284MB24965479BA59DF5C2CC01CA0EC462@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
 <20240206215022.GA3452@nathanxps13>
 <CP8P284MB2496A17A9852265A0F363A6AEC452@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
 <852cabef-f596-4826-9a54-d3f75d9a6cc8@postgrespro.ru>
 <CP8P284MB24961361A9C1D32C28A7EC0EEC452@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
 <e289dada-543d-495f-9cd3-5783a8412bc0@postgrespro.ru>
 <CP4P284MB24830F5BF17FFA7F3D733142EC452@CP4P284MB2483.BRAP284.PROD.OUTLOOK.COM>
 <47dcfd6f-d1d8-4a13-9bc5-b43ee9bd534f@uni-muenster.de>
 <CP8P284MB2496D053FD15C80780B72F46EC442@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
From: Jim Jones <jim.jones@uni-muenster.de>
In-Reply-To: <CP8P284MB2496D053FD15C80780B72F46EC442@CP8P284MB2496.BRAP284.PROD.OUTLOOK.COM>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Archived-At: <https://www.postgresql.org/message-id/cefe290e-4e70-4cfd-89e9-888e916f6b51%40uni-muenster.de>
Precedence: bulk


On 08.02.24 16:50, Maiquel Grassi wrote:
> 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 "postgres" as user "maiquel" 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 | maiquel            |             | maiquel      | maiquel
>      |       31485 | 17devel        | ::1            | 5432        |
> ::1            |       47482 |                  | localhost
> (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
>     |       31488 | 17devel        | ::1            | 5432        |
> ::1            |       47484 |                  | localhost
> (1 row)
>
> Regards,
> Maiquel.

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!

-- 
Jim