public inbox for [email protected]  
help / color / mirror / Atom feed
Re: Psql meta-command conninfo+
6+ messages / 4 participants
[nested] [flat]

* Re: Psql meta-command conninfo+
@ 2025-01-10 07:47  Hunaid Sohail <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Hunaid Sohail @ 2025-01-10 07:47 UTC (permalink / raw)
  To: Sami Imseih <[email protected]>; +Cc: David G. Johnston <[email protected]>; Alvaro Herrera <[email protected]>; Maiquel Grassi <[email protected]>; pgsql-hackers; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Peter Eisentraut <[email protected]>; Pavel Luzanov <[email protected]>; Erik Wienhold <[email protected]>

Hi,

On Fri, Jan 10, 2025 at 1:50 AM Sami Imseih <[email protected]> wrote:

> > If we go with the 3 column format, then we will just
> > have a bunch of repeated values for Category, which
> > looks cluttered, IMO.
>
> "cluttered" is maybe the wrong description. I meant the output
> will look overwhelming due to the repeated values :)
>

+1.

To summarize:

- Alvaro is not happy with \conninfo+ printing multiple tables and suggests
we continue with v35.

- Sami is ok with both approaches but thinks that \conninfo+ should also
print all parameters from PQparameterStatus in a dynamic way. Also, he
suggests to include "role" if we are showing is_superuser.
For that, we need two more patches:
1. Mark role as GUC_REPORT with necessary doc changes.
2. Add a new libpq function (perhaps PQparameterNames) to return the names
of parameters, so we can also display parameters added in the future,
along with doc updates.

- David wants category information included.

IMO, we should continue with v35 and add all parameters from
PQparameterStatus,
as Sami suggested. The names themselves are informative enough.

Looking forward to your feedback.

Regards,
Hunaid Sohail


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Psql meta-command conninfo+
@ 2025-01-13 08:44  Alvaro Herrera <[email protected]>
  parent: Hunaid Sohail <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Alvaro Herrera @ 2025-01-13 08:44 UTC (permalink / raw)
  To: Hunaid Sohail <[email protected]>; +Cc: Sami Imseih <[email protected]>; David G. Johnston <[email protected]>; Maiquel Grassi <[email protected]>; pgsql-hackers; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Peter Eisentraut <[email protected]>; Pavel Luzanov <[email protected]>; Erik Wienhold <[email protected]>

On 2025-Jan-10, Hunaid Sohail wrote:

> IMO, we should continue with v35 and add all parameters from
> PQparameterStatus,
> as Sami suggested. The names themselves are informative enough.

IMO we need more opinions.  Maybe enough other people are not as unhappy
about the three-table solution.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Psql meta-command conninfo+
@ 2025-01-13 11:12  Dean Rasheed <[email protected]>
  parent: Alvaro Herrera <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Dean Rasheed @ 2025-01-13 11:12 UTC (permalink / raw)
  To: Alvaro Herrera <[email protected]>; +Cc: Hunaid Sohail <[email protected]>; Sami Imseih <[email protected]>; David G. Johnston <[email protected]>; Maiquel Grassi <[email protected]>; pgsql-hackers; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Peter Eisentraut <[email protected]>; Pavel Luzanov <[email protected]>; Erik Wienhold <[email protected]>

On Mon, 13 Jan 2025 at 08:44, Alvaro Herrera <[email protected]> wrote:
>
> > IMO, we should continue with v35 and add all parameters from
> > PQparameterStatus,
> > as Sami suggested. The names themselves are informative enough.
>
> IMO we need more opinions.  Maybe enough other people are not as unhappy
> about the three-table solution.
>

I don't like the 3-table format either. I think it should be a single table.

The trouble with v35 is that it produces 1 row with lots of columns,
which is pretty unreadable unless expanded mode is used. So I think we
should just do it that way round by default -- i.e., make it like
\dconfig and have 2 columns, "Parameter" and "Value", with lots of
rows.

Perhaps it could also have a "Description" column, which might help
with things like distinguishing between authenticated user and session
user.

Regards,
Dean






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Psql meta-command conninfo+
@ 2025-01-14 07:57  Hunaid Sohail <[email protected]>
  parent: Dean Rasheed <[email protected]>
  0 siblings, 1 reply; 6+ messages in thread

From: Hunaid Sohail @ 2025-01-14 07:57 UTC (permalink / raw)
  To: Dean Rasheed <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Sami Imseih <[email protected]>; David G. Johnston <[email protected]>; Maiquel Grassi <[email protected]>; pgsql-hackers; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Peter Eisentraut <[email protected]>; Pavel Luzanov <[email protected]>; Erik Wienhold <[email protected]>

Hi,

On Mon, Jan 13, 2025 at 4:12 PM Dean Rasheed <[email protected]>
wrote:

> I don't like the 3-table format either. I think it should be a single
> table.
>
> The trouble with v35 is that it produces 1 row with lots of columns,
> which is pretty unreadable unless expanded mode is used. So I think we
> should just do it that way round by default -- i.e., make it like
> \dconfig and have 2 columns, "Parameter" and "Value", with lots of
> rows.
>
> Perhaps it could also have a "Description" column, which might help
> with things like distinguishing between authenticated user and session
> user.
>

I've tried the approach and attached the output.
I'm not attaching the patch as it requires some formatting. Does this look
good?
I have added a one liner description that is consistent with libpq docs.

postgres=# \conninfo+
                                                     Connection Information
      Parameter       |         Value          |
       Description
----------------------+------------------------+--------------------------------------------------------------------------------
 Database             | postgres               | Displays the database name
of the connection.
 Client User          | hunaid                 | Displays the name of the
user connected to the database as returned by PQuser.
 Host                 | localhost              | Displays the server host
name of the active connection.
 Host Address         | 127.0.0.1              | Displays the server IP
address of the active connection.
 Port                 | 5430                   | Displays the port number
used for the database connection.
 Options              |                        | Displays any command-line
options passed in the connection request.
 Protocol Version     | 3                      | Displays the
frontend/backend protocol being used.
 Password Used        | false                  | Indicates whether a
password was used during authentication.
 GSSAPI Authenticated | false                  | Indicates if GSSAPI
authentication was used for the connection.
 Backend PID          | 43465                  | Displays the process ID of
the backend for the connection.
 SSL Connection       | true                   | Indicates whether SSL
encryption is in use for the connection.
 SSL Protocol         | TLSv1.3                | Displays the SSL protocol
used for the connection.
 SSL Cipher           | TLS_AES_256_GCM_SHA384 | Displays the SSL cipher
used for the connection.
 SSL Compression      | off                    | Indicates whether SSL
compression is enabled.
 ALPN                 | postgresql             | Displays the ALPN protocol
used in the SSL connection.
(15 rows)

I was wondering how we can show the descriptions of parameters
from PQparameterStatus. We can create an array of descriptions,
but if the order changes somehow, it can mess things up.

Looking forward to your opinions. Can we continue with this approach?

Regards,
Hunaid Sohail


^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* Re: Psql meta-command conninfo+
@ 2025-01-14 08:51  Alvaro Herrera <[email protected]>
  parent: Hunaid Sohail <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Alvaro Herrera @ 2025-01-14 08:51 UTC (permalink / raw)
  To: Hunaid Sohail <[email protected]>; +Cc: Dean Rasheed <[email protected]>; Sami Imseih <[email protected]>; David G. Johnston <[email protected]>; Maiquel Grassi <[email protected]>; pgsql-hackers; Jim Jones <[email protected]>; Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Peter Eisentraut <[email protected]>; Pavel Luzanov <[email protected]>; Erik Wienhold <[email protected]>

On 2025-Jan-14, Hunaid Sohail wrote:

> I'm not attaching the patch as it requires some formatting.

Remember pgindent formats code in bulk.  A quick useful workflow is to
do that first, apply manual adjustments next, run pgindent again
afterwards.

> I've tried the approach and attached the output.  Does this look good?

Hmm, I'm not sure I like the third column particularly; it's noisy to
have on all the time.  I'd leave that for \conninfo++ maybe :-)

_If_ we keep the descriptions, I suggest that the majority need to be
very short, say three words tops, with exceptions allowed for egregious
cases (if any).  We should get rid of filler words such as "Displays
the" or "Indicates whether".  For instance, I'd turn "Indicates if
GSSAPI authentication was used for the connection" into "GSSAPI
authenticated?" which conveys exactly the same; the ending "?" matches
things like "Key?" in \d of an index or the "Default?" column in \dc.
It's a boolean, so people know how to read it.

> I was wondering how we can show the descriptions of parameters
> from PQparameterStatus. We can create an array of descriptions,
> but if the order changes somehow, it can mess things up.

Definitely avoid writing code that can easily be broken by later changes
(future parameter additions and so on).

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"The problem with the facetime model is not just that it's demoralizing, but
that the people pretending to work interrupt the ones actually working."
                  -- Paul Graham, http://www.paulgraham.com/opensource.html






^ permalink  raw  reply  [nested|flat] 6+ messages in thread

* [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B.
@ 2026-03-11 13:53  Antonin Houska <[email protected]>
  0 siblings, 0 replies; 6+ messages in thread

From: Antonin Houska @ 2026-03-11 13:53 UTC (permalink / raw)

Since VARSIZE_ANY() may call VARSIZE_4B(), it's possible that the compiler
(when invoked with -Warray-bounds) complains if the argument of VARSIZE_ANY()
is actually smaller than what VARSIZE_4B() expects. This patch adjusts the
VARSIZE_4B() macro so that it does not have to dereference the varattrib_4b
structure. We assume that varlena value always starts with the length word.

The problem does not exist in the tree at the moment since the current users
of VARSIZE_ANY() pass a pointer to a dynamically allocated memory, so the
compiler has no idea about the memory available. However, in an upcoming
patch, it makes sense to pass a pointer to a local variable of "varlena"
type. In such a case, the compiler warning might appear because
sizeof(varlena) is lower than sizeof(varattrib_4b).
---
 src/include/varatt.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/include/varatt.h b/src/include/varatt.h
index 000bdf33b92..31063e5d4f1 100644
--- a/src/include/varatt.h
+++ b/src/include/varatt.h
@@ -207,7 +207,7 @@ typedef struct
 
 /* VARSIZE_4B() should only be used on known-aligned data */
 #define VARSIZE_4B(PTR) \
-	(((const varattrib_4b *) (PTR))->va_4byte.va_header & 0x3FFFFFFF)
+	(*((const uint32 *) (PTR)) & 0x3FFFFFFF)
 #define VARSIZE_1B(PTR) \
 	(((const varattrib_1b *) (PTR))->va_header & 0x7F)
 #define VARTAG_1B_E(PTR) \
@@ -240,7 +240,7 @@ typedef struct
 
 /* VARSIZE_4B() should only be used on known-aligned data */
 #define VARSIZE_4B(PTR) \
-	((((const varattrib_4b *) (PTR))->va_4byte.va_header >> 2) & 0x3FFFFFFF)
+	((*((const uint32 *) (PTR)) >> 2) & 0x3FFFFFFF)
 #define VARSIZE_1B(PTR) \
 	((((const varattrib_1b *) (PTR))->va_header >> 1) & 0x7F)
 #define VARTAG_1B_E(PTR) \
-- 
2.47.3


--=-=-=
Content-Type: text/plain
Content-Disposition: attachment;
 filename=v42-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch



^ permalink  raw  reply  [nested|flat] 6+ messages in thread


end of thread, other threads:[~2026-03-11 13:53 UTC | newest]

Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-01-10 07:47 Re: Psql meta-command conninfo+ Hunaid Sohail <[email protected]>
2025-01-13 08:44 ` Alvaro Herrera <[email protected]>
2025-01-13 11:12   ` Dean Rasheed <[email protected]>
2025-01-14 07:57     ` Hunaid Sohail <[email protected]>
2025-01-14 08:51       ` Alvaro Herrera <[email protected]>
2026-03-11 13:53 [PATCH 2/7] Do not dereference varattrib_4b in VARSIZE_4B. Antonin Houska <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox