Subject: Re: Adding comments to help understand psql hidden queries

From: David Christensen <[email-masked]>
Date: Thu, 1 Feb 2024 16:39:08 -0600

On Thu, Feb 1, 2024 at 4:34 PM Greg Sabino Mullane <[email-masked]> wrote:
>
> The use of the --echo-hidden flag in psql is used to show people the way psql performs its magic for its backslash commands. None of them has more magic than "\d relation", but it suffers from needing a lot of separate queries to gather all of the information it needs. Unfortunately, those queries can get overwhelming and hard to figure out which one does what, especially for those not already very familiar with the system catalogs. Attached is a patch to add a small SQL comment to the top of each SELECT query inside describeOneTableDetail. All other functions use a single query, and thus need no additional context. But "\d mytable" has the potential to run over a dozen SQL queries! The new format looks like this:
>
> /******** QUERY *********/
> /* Get information about row-level policies */
> SELECT pol.polname, pol.polpermissive,
>   CASE WHEN pol.polroles = '{0}' THEN NULL ELSE pg_catalog.array_to_string(array(select rolname from pg_catalog.pg_roles where oid = any (pol.polroles) order by 1),',') END,
>   pg_catalog.pg_get_expr(pol.polqual, pol.polrelid),
>   pg_catalog.pg_get_expr(pol.polwithcheck, pol.polrelid),
>   CASE pol.polcmd
>     WHEN 'r' THEN 'SELECT'
>     WHEN 'a' THEN 'INSERT'
>     WHEN 'w' THEN 'UPDATE'
>     WHEN 'd' THEN 'DELETE'
>     END AS cmd
> FROM pg_catalog.pg_policy pol
> WHERE pol.polrelid = '134384' ORDER BY 1;
> /************************/
>
> Cheers,
> Greg

Thanks, this looks like some helpful information. In the same vein,
I'm including a patch which adds information about the command that
generates the given query as well (atop your commit).  This will
modify the query line to include the command itself:

/******** QUERY (\dRs) *********/

Best,

David

---

From: Jim Jones <[email-masked]>
Date: Fri, 15 Mar 2024 14:21:59 +0100

Hi Greg, hi David

> /******** QUERY (\dRs) *********/

Having this kind of information in each query would have saved me a lot
of time in the past :) +1

There is a tiny little issue in the last patch (qualifiers):

command.c:312:16: warning: assignment discards ‘const’ qualifier from
pointer target type [-Wdiscarded-qualifiers]
  312 |         curcmd = cmd;

Thanks

-- 
Jim

---

From: David Christensen <[email-masked]>
Date: Thu, 21 Mar 2024 12:31:42 -0500

Hi Jim,

Thanks for the feedback.  Enclosed is a v2 of this series(?) rebased
and with that warning fixed; @Greg Sabino Mullane I just created a
commit on your behalf with the message to hackers.  I'm also creating
a commit-fest entry for this thread.

Best,

David

---

From: David Christensen <[email-masked]>
Date: Thu, 21 Mar 2024 18:14:26 +0000

Created the CF entry in commitfest 48 but didn't see it was already in 47; closing the CFEntry in 48. (Doesn't appear to be a different status than "Withdrawn"...)

---

From: Peter Eisentraut <[email-masked]>
Date: Thu, 21 Mar 2024 23:20:05 +0100

> a commit-fest entry for this thread.

I don't think your patch takes into account that the

/**... QUERY ...**/
...
/**...       ...**/

lines are supposed to align vertically.  With your patch, the first line 
would have variable length depending on the command.

---

From: Greg Sabino Mullane <[email-masked]>
Date: Fri, 22 Mar 2024 10:46:48 -0400

> would have variable length depending on the command.

Yes, that is a good point. Aligning those would be quite tricky, what if we
just kept a standard width for the closing query? Probably the 24 stars we
currently have to match "QUERY", which it appears nobody has changed for
translation purposes yet anyway. (If I am reading the code correctly, it
would be up to the translators to maintain the vertical alignment).

Cheers,
Greg