Thread
Commits
-
Add labels to help make psql's hidden queries more understandable.
- 41d69e6dccad 19 (unreleased) landed
-
Adding comments to help understand psql hidden queries
Greg Sabino Mullane <htamfids@gmail.com> — 2023-12-11T21:53:01Z
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 -
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-02-01T22:39:08Z
On Thu, Feb 1, 2024 at 4:34 PM Greg Sabino Mullane <htamfids@gmail.com> 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 -
Re: Adding comments to help understand psql hidden queries
Jim Jones <jim.jones@uni-muenster.de> — 2024-03-15T13:21:59Z
Hi Greg, hi David On 01.02.24 23:39, David Christensen wrote: > On Thu, Feb 1, 2024 at 4:34 PM Greg Sabino Mullane <htamfids@gmail.com> 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 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 -
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-03-21T17:31:42Z
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
-
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-03-21T18:14:26Z
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"...)
-
Re: Adding comments to help understand psql hidden queries
Peter Eisentraut <peter@eisentraut.org> — 2024-03-21T22:20:05Z
On 21.03.24 18:31, David Christensen wrote: > 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. 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.
-
Re: Adding comments to help understand psql hidden queries
Greg Sabino Mullane <htamfids@gmail.com> — 2024-03-22T14:46:48Z
On Thu, Mar 21, 2024 at 6:20 PM Peter Eisentraut <peter@eisentraut.org> wrote: > lines are supposed to align vertically. With your patch, the first line > 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
-
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-03-22T15:39:10Z
On Fri, Mar 22, 2024 at 9:47 AM Greg Sabino Mullane <htamfids@gmail.com> wrote: > > On Thu, Mar 21, 2024 at 6:20 PM Peter Eisentraut <peter@eisentraut.org> wrote: >> >> lines are supposed to align vertically. With your patch, the first line >> 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). I think it's easier to keep the widths balanced than constant (patch version included here), but if we needed to squeeze the opening string to a standard width that would be possible without too much trouble. The internal comment strings seem to be a bit more of a pain if we wanted all of the comments to be the same width, as we'd need a table or something so we can compute the longest string width, etc; doesn't seem worth the convolutions IMHO. No changes to Greg's patch, just keeping 'em both so cfbot stays happy. David
-
Re: Adding comments to help understand psql hidden queries
Greg Sabino Mullane <htamfids@gmail.com> — 2024-03-22T17:37:48Z
On Fri, Mar 22, 2024 at 11:39 AM David Christensen <david+pg@pgguru.net> wrote: > I think it's easier to keep the widths balanced than constant (patch > version included here) Yeah, I'm fine with that, especially because nobody is translating it, nor are they likely to, to be honest. Cheers, Greg
-
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-04-03T17:16:05Z
I got Greg's blessing on squashing the commits down, and now including a v4 with additional improvements on the output formatting front. Main changes: - all generated comments are the same width - width has been bumped to 80 - removed _() functions for consumers of the new output functions This patch adds two new helper functions, OutputComment() and OutputCommentStars() to output and wrap the comments to the appropriate widths. Everything should continue to work just fine if we want to adjust the width by just adjusting the MAX_COMMENT_WIDTH symbol. I removed _() in the output of the query/stars since there'd be no sensible existing translations for the constructed string, which included the query string itself. If we need it for the "QUERY" string, this could be added fairly easily, but the existing piece would have been nonsensical and never used in practice. Thanks, David
-
Re: Adding comments to help understand psql hidden queries
Peter Eisentraut <peter@eisentraut.org> — 2024-04-04T14:31:58Z
On 03.04.24 19:16, David Christensen wrote: > I removed _() in the output of the query/stars since there'd be no > sensible existing translations for the constructed string, which > included the query string itself. If we need it for the "QUERY" > string, this could be added fairly easily, but the existing piece > would have been nonsensical and never used in practice. "QUERY" is currently translated. Your patch loses that.
-
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-04-04T16:12:18Z
On Thu, Apr 4, 2024 at 9:32 AM Peter Eisentraut <peter@eisentraut.org> wrote: > > On 03.04.24 19:16, David Christensen wrote: > > I removed _() in the output of the query/stars since there'd be no > > sensible existing translations for the constructed string, which > > included the query string itself. If we need it for the "QUERY" > > string, this could be added fairly easily, but the existing piece > > would have been nonsensical and never used in practice. > > "QUERY" is currently translated. Your patch loses that. I see; enclosed is v5 which fixes this. The effective diff from the last one is: - char *label = "QUERY"; + char *label = _("QUERY"); and - label = psprintf("QUERY (\\%s)", curcmd); + label = psprintf(_("QUERY (\\%s)"), curcmd); Best, David -
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2024-06-11T22:54:34Z
On Thu, Apr 4, 2024 at 11:12 AM David Christensen <david+pg@pgguru.net> wrote: > > On Thu, Apr 4, 2024 at 9:32 AM Peter Eisentraut <peter@eisentraut.org> wrote: > > > > On 03.04.24 19:16, David Christensen wrote: > > > I removed _() in the output of the query/stars since there'd be no > > > sensible existing translations for the constructed string, which > > > included the query string itself. If we need it for the "QUERY" > > > string, this could be added fairly easily, but the existing piece > > > would have been nonsensical and never used in practice. > > > > "QUERY" is currently translated. Your patch loses that. > > I see; enclosed is v5 which fixes this. > > The effective diff from the last one is: > > - char *label = "QUERY"; > + char *label = _("QUERY"); > > and > > - label = psprintf("QUERY (\\%s)", curcmd); > + label = psprintf(_("QUERY (\\%s)"), curcmd); Any further concerns/issues with this patch that I can address to help move it forward? David -
Re: Adding comments to help understand psql hidden queries
Tom Lane <tgl@sss.pgh.pa.us> — 2025-01-18T20:37:54Z
David Christensen <david+pg@pgguru.net> writes: > Any further concerns/issues with this patch that I can address to help > move it forward? I got around to looking at this finally --- sorry that it's been on the back burner for so long. I think this is basically a good idea but it still requires a lot of sanding-down of rough edges. The patch doesn't apply cleanly anymore, which is unsurprising since it's been sitting for months; what might be more surprising is that there was only one hunk that had to be fixed by hand. I noticed also that "git diff --check" complains about a bunch of whitespace style violations, and that brace layout and comment layout largely fail to comply with PG project standards. I ran the patched code through pgindent to get rid of those warnings, but did not really look at whether any of what it changed could be done better. (I attach a v6 with the results of those changes to pacify the cfbot. I have not made any changes responding to my comments below.) Playing around with what it does, my first observation is that the results look absolutely horrid in an 80-column xterm window: $ psql -E regression psql (18devel) Type "help" for help. regression=# \d tenk1 /********************************* QUERY (\d) ********************************** / SELECT c.oid, ... ORDER BY 2, 3; /******************************************************************************* / /********************************* QUERY (\d) ********************************** / /* Get general table information * / SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, c.relhastriggers, ... So evidently the effective line length is 81 characters not the 80 that the code claims to be using. I did not look to see where the off-by-one error is. On the particular xterm setup I use, backing off the number of stars by one would be enough to make that look better; but I have very often used setups where printing 80 characters and a newline would result in a blank line. I think the comment width must be reduced to no more than 79 characters. Even that seems a little questionable; are there people who use less-than-80-column terminal windows? I think aiming for 60 or so columns might be smarter. There's another issue here too, arising from the fact that you want to give translated strings to OutputComment(). That's laudable, but it means that strlen() isn't even approximately the right computation for how many columns the string will occupy on-screen. (There are very likely some multibyte characters in the translated string, and then again some of those characters could be double-width ideograms.) Now psql does contain code that can compute the actual displayed width of a translated string, but frankly I'm beginning to question the value of the whole business. How about just printing a fixed number of stars, like ten, and dropping the whole concept of a target line length? /********** QUERY (\d) **********/ /* Get general table information */ ... blah blah blah ... /*********************/ Secondly, looking at the whole output, it seems quite repetitive: regression=# \d tenk1 /********************************* QUERY (\d) **********************************/ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get general table information */ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get information about each column */ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get information about each index */ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get information about row-level policies */ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get information about extended statistics */ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get information about each publication using this table */ SELECT ... /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ SELECT c.oid::pg_catalog.regclass FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i WHERE c.oid = i.inhparent AND i.inhrelid = '16418' AND c.relkind != 'p' AND c.relkind != 'I' ORDER BY inhseqno; /*******************************************************************************/ /********************************* QUERY (\d) **********************************/ /* Get information about child tables */ SELECT ... /*******************************************************************************/ Surely we do not need to repeat the "QUERY (\d)" line; in fact, I think it's confusing to do so. That should appear but once per user command. I also find all the stars to be fairly visually distracting. What do you think of losing those altogether in favor of blank lines? Something like /********** QUERY (\d) **********/ SELECT ... /* Get general table information */ SELECT ... /* Get information about each column */ SELECT ... /* Get information about each index */ SELECT ... /* Get information about row-level policies */ SELECT ... /* Get information about extended statistics */ SELECT ... /* Get information about each publication using this table */ SELECT ... /* Get information about child tables */ SELECT ... Maybe that's too far in the other direction, but it seems worth thinking about. (BTW, there is one query in the output for \d that lacks an OutputComment gloss. Maybe that's one that got added since this patch was written?) Moving on to actual code review: * The "curcmd" global variable is quite horrid IMO. It would be only slightly less horrid if it were properly documented and declared in a header file as globals should be. I suppose you did that to avoid having to pass the command string down through a ton of subroutines. However, with my proposal that the query should be printed only once at the start, maybe we could relocate the responsibility for printing it to somewhere closer to exec_command(), and thus reduce the notational overhead? * It took me awhile to realize that OutputComment resets the target buffer while OutputCommentStars doesn't. Neither their names nor their header comments give any clue about that rather critical-to-callers property. I don't like the name "OutputCommentStars" anyway as it puts emphasis on what it should be *hiding* from callers, namely the form of the output. Not sure about a better name. * Enabling log_statement = 'all' proves that the comments added by OutputComment are sent to the server, *even when -E is not on*: 2025-01-18 15:09:23.575 EST [2587557] LOG: statement: /* Get general table information */ SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, false AS relhasoids, c.relispartition, '', c.reltablespace, CASE WHEN c.reloftype = 0 THEN '' ELSE c.reloftype::pg_catalog.regtype::pg_catalog.text END, c.relpersistence, c.relreplident, am.amname FROM pg_catalog.pg_class c LEFT JOIN pg_catalog.pg_class tc ON (c.reltoastrelid = tc.oid) LEFT JOIN pg_catalog.pg_am am ON (c.relam = am.oid) WHERE c.oid = '16418'; I don't find that acceptable at all. For one thing, it makes the stakes extremely high (as in possibly security-critical) that there are no "*/" sequences in the label strings. On the whole I'd rather arrange things so that the comments are only emitted to the psql terminal and never sent to the server. It appears that that's true already for some of them --- I didn't trouble to try to understand why these behave differently. * In connection with that, I'm none too comfortable with the assumption "there are no inner comments that need to be escaped", mainly for the comments that include fragments of user queries. If we can ensure that none of this output gets to the server then maybe it's not too critical, but I'm not really convinced. Is it worth doing something to sanitize the comment contents? * I think the \n here was unintended: + OutputComment(&buf, _("Get information about each column\n")); That leads to some oddly-formatted output. Anyway, I encourage you to work on these issues and see if we can get to a committable patch. regards, tom lane -
Re: Adding comments to help understand psql hidden queries
David Christensen <david+pg@pgguru.net> — 2025-01-30T20:55:00Z
On Sat, Jan 18, 2025 at 2:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > David Christensen <david+pg@pgguru.net> writes: > > Any further concerns/issues with this patch that I can address to help > > move it forward? > > I got around to looking at this finally --- sorry that it's been on > the back burner for so long. I think this is basically a good idea > but it still requires a lot of sanding-down of rough edges. Hi Tom, thanks for the detailed feedback. I'll take your v6 and see about adding the additional changes; I agree with what you've pointed out at the high level, and will respond with additional questions of my own if things seem ambiguous in terms of approach. Thanks, David
-
Re: Adding comments to help understand psql hidden queries
Maiquel Grassi <grassi@hotmail.com.br> — 2025-04-01T00:02:25Z
Hi! I have read the discussion and would like to share my humble opinion. I believe that a visually appealing way to display the output on the screen is to ensure symmetry in the length of asterisks and description lines. I imagine someone looking at the screen and focusing on symmetrical details. Therefore, the string length should serve as the basis for the calculation. If the description length is an even number, then the formula would be: ((description length − 7) / 2) Placing this result of asterisks on both sides of the string ' QUERY ' ensures balance. If the description length is an odd number, then place: ((description length − 7) / 2) asterisks on the right side and: (((description length − 7) / 2) + 1) asterisks on the left side. This method does not always result in a perfectly symmetric number of asterisks, but it provides a more visually aligned appearance. At the end of the SQL code, we should also include a line terminator of the same length of the description. The 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; /********************************************/ Regards, Maiquel. -
Re: Adding comments to help understand psql hidden queries
Greg Sabino Mullane <htamfids@gmail.com> — 2026-03-22T15:15:38Z
Going back through all the feedback and comments, plus having some time to think things through, I am including a new patch, v7, that greatly simplifies things, and only makes changes inside of describe.c. In the spirit of not letting the perfect be the enemy of the good, I'm not worrying at all about the number of stars, or the width, and simply adding a small consistent description at the top of each query. I also realized that having these queries show up in someone's server log could be quite confusing, so I had them output as part of the query itself. In other words, they show up in both psql -E and the server logs. A few benefits to doing this: * Simplifies the code * Makes searching the web for what generated this code a lot easier (a comment versus a giant blob of SQL) * Makes all the SQL a little bit self-documented everywhere it shows up * Easier to maintain describe.c, as the comment is always printfPQExpBuffer, and everything else is appendPQExpBuffer, rather than trying to figure out which to use for each section of SQL. Also removes bugs like the append-first in objectDescription() Here's what the new output looks like via psql -E: /******** QUERY *********/ /* Get matching aggregates */ SELECT n.nspname as "Schema", p.proname AS "Name", pg_catalog.format_type(p.prorettype, NULL) AS "Result data type", CASE WHEN p.pronargs = 0 THEN CAST('*' AS pg_catalog.text) ELSE pg_catalog.pg_get_function_arguments(p.oid) END AS "Argument data types", pg_catalog.obj_description(p.oid, 'pg_proc') as "Description" FROM pg_catalog.pg_proc p LEFT JOIN pg_catalog.pg_namespace n ON n.oid = p.pronamespace WHERE p.prokind = 'a' AND n.nspname <> 'pg_catalog' AND n.nspname <> 'information_schema' AND pg_catalog.pg_function_is_visible(p.oid) ORDER BY 1, 2, 4; /************************/ and more examples: /******** QUERY *********/ /* Get publications that exclude this table */ SELECT pubname FROM pg_catalog.pg_publication p JOIN pg_catalog.pg_publication_rel pr ON p.oid = pr.prpubid WHERE (pr.prrelid = '16403' OR pr.prrelid = pg_catalog.pg_partition_root('16403')) AND pr.prexcept ORDER BY 1; /************************/ /******** QUERY *********/ /* Get parent tables */ SELECT c.oid::pg_catalog.regclass FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i WHERE c.oid = i.inhparent AND i.inhrelid = '16403' AND c.relkind != 'p' AND c.relkind != 'I' ORDER BY inhseqno; /************************/ Cheers, Greg -
Re: Adding comments to help understand psql hidden queries
Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-23T17:35:04Z
Greg Sabino Mullane <htamfids@gmail.com> writes: > Going back through all the feedback and comments, plus having some time to > think things through, I am including a new patch, v7, that greatly > simplifies things, and only makes changes inside of describe.c. In the > spirit of not letting the perfect be the enemy of the good, I'm not > worrying at all about the number of stars, or the width, and simply adding > a small consistent description at the top of each query. I also realized > that having these queries show up in someone's server log could be quite > confusing, so I had them output as part of the query itself. In other > words, they show up in both psql -E and the server logs. I like this proposal quite a lot. It seems like about the right level of development/maintenance effort, and I think it provides a useful increment of usability. I have one slightly-orthogonal suggestion. I think that we should make the header for generated queries be different from that used for user queries in psql's logfile mode. Right now those are both /******** QUERY *********/ I propose instead printing /**** INTERNAL QUERY ****/ if it's a generated query. I went through the 0007 patch and made some editorial changes beyond that addition. I fixed a couple more internal queries to have headers (now each PSQLexec in describe.c has one). I also changed some of Greg's proposed headers in hopes of improving uniformity. Notably, I didn't like that some of the headers said "table" and some said "relation". I made them all say "table", although you could certainly argue for the opposite. This is all in-the-eye-of-the-beholder, so feel free to push back on any changes you don't like. For ease of review, v8-0001 attached is identical to Greg's 0007, and v8-0002 is my proposed changes on top of that. regards, tom lane
-
Re: Adding comments to help understand psql hidden queries
Greg Sabino Mullane <htamfids@gmail.com> — 2026-03-23T22:50:16Z
Thanks for looking this over. I'm pretty happy with the patch as is now. I agree the INTERNAL QUERY is a nice touch. I once thought about adding "psql" into the header somehow as a kind of application_name self labelling, but I think INTERNAL QUERY will be distinct enough. Notably, I didn't like that some of the headers said "table" and some said > "relation". I made them all say "table", although you could certainly > argue for the opposite. I originally had "table", but then it felt weird in my testing when I was describing a sequence or view it said table. So I'm a weak +1 for relation. -- Cheers, Greg
-
Re: Adding comments to help understand psql hidden queries
Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-23T23:03:41Z
Greg Sabino Mullane <htamfids@gmail.com> writes: >> Notably, I didn't like that some of the headers said "table" and some said >> "relation". I made them all say "table", although you could certainly >> argue for the opposite. > I originally had "table", but then it felt weird in my testing when I was > describing a sequence or view it said table. So I'm a weak +1 for relation. My preference for "table" is likewise weak. Anyone else have an opinion? regards, tom lane
-
Re: Adding comments to help understand psql hidden queries
Tom Lane <tgl@sss.pgh.pa.us> — 2026-03-26T15:41:53Z
I wrote: > Greg Sabino Mullane <htamfids@gmail.com> writes: >>> Notably, I didn't like that some of the headers said "table" and some said >>> "relation". I made them all say "table", although you could certainly >>> argue for the opposite. >> I originally had "table", but then it felt weird in my testing when I was >> describing a sequence or view it said table. So I'm a weak +1 for relation. > My preference for "table" is likewise weak. Anyone else have an > opinion? [ crickets... ] After sleeping on it and taking another look at the output, I agree that we need to use a mix of "relation" and "table", because some of these queries definitely apply to all kinds of pg_class entries, while for others we must be dealing with a table (or something reasonably table-like, such as a foreign table). I made another pass over it to fix that, and pushed the results. Thanks for working on this! I know it's been a long process, but sometimes that's what it takes to get to a consensus. regards, tom lane