public inbox for [email protected]
help / color / mirror / Atom feedRe: Adding comments to help understand psql hidden queries
12+ messages / 5 participants
[nested] [flat]
* Re: Adding comments to help understand psql hidden queries
@ 2024-03-22 15:39 David Christensen <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David Christensen @ 2024-03-22 15:39 UTC (permalink / raw)
To: Greg Sabino Mullane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
On Fri, Mar 22, 2024 at 9:47 AM Greg Sabino Mullane <[email protected]> wrote:
>
> On Thu, Mar 21, 2024 at 6:20 PM Peter Eisentraut <[email protected]> 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
Attachments:
[application/octet-stream] v3-0002-Add-output-of-the-command-that-got-us-here-to-the.patch (2.9K, 2-v3-0002-Add-output-of-the-command-that-got-us-here-to-the.patch)
download | inline diff:
From 83f2586c6a28f237a8e81a9d846b44975998307e Mon Sep 17 00:00:00 2001
From: David Christensen <[email protected]>
Date: Thu, 21 Mar 2024 12:27:32 -0500
Subject: [PATCH v3 2/2] Add output of the command that got us here to the
QUERY output
---
src/bin/psql/command.c | 5 +++++
src/bin/psql/common.c | 24 ++++++++++++++++++++----
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9b0fa041f7..4cfb91e134 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -56,6 +56,8 @@ typedef enum EditableObjectType
EditableView,
} EditableObjectType;
+char *curcmd = NULL;
+
/* local function declarations */
static backslashResult exec_command(const char *cmd,
PsqlScanState scan_state,
@@ -307,6 +309,7 @@ exec_command(const char *cmd,
cmd);
}
+ curcmd = (char *)cmd;
if (strcmp(cmd, "a") == 0)
status = exec_command_a(scan_state, active_branch);
else if (strcmp(cmd, "bind") == 0)
@@ -423,6 +426,8 @@ exec_command(const char *cmd,
else
status = PSQL_CMD_UNKNOWN;
+ curcmd = NULL;
+
/*
* All the commands that return PSQL_CMD_SEND want to execute previous_buf
* if query_buf is empty. For convenience we implement that here, not in
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 76e01b02a3..18c8b067ee 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -42,6 +42,7 @@ static int ExecQueryAndProcessResults(const char *query,
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
+extern char *curcmd;
/*
* openQueryOutputFile --- attempt to open a query output file
@@ -581,6 +582,12 @@ PGresult *
PSQLexec(const char *query)
{
PGresult *res;
+ char *label = "";
+ const char *asterisks = "********************"; /* ensure this is as long as the
+ * longest command that might be
+ * displayed */
+ int curcmd_length = 0;
+
if (!pset.db)
{
@@ -588,21 +595,30 @@ PSQLexec(const char *query)
return NULL;
}
+ if (curcmd)
+ {
+ label = psprintf(" (\\%s)", curcmd);
+ curcmd_length = strlen(label);
+ }
+
if (pset.echo_hidden != PSQL_ECHO_HIDDEN_OFF)
{
- printf(_("/******** QUERY *********/\n"
+ printf(_("/******** QUERY%s *********/\n"
"%s\n"
- "/************************/\n\n"), query);
+ "/************************%.*s/\n\n"), label, query, curcmd_length, asterisks);
fflush(stdout);
if (pset.logfile)
{
fprintf(pset.logfile,
- _("/******** QUERY *********/\n"
+ _("/******** QUERY%s *********/\n"
"%s\n"
- "/************************/\n\n"), query);
+ "/************************%.*s/\n\n"), label, query, curcmd_length, asterisks);
fflush(pset.logfile);
}
+ if (curcmd)
+ pfree(label);
+
if (pset.echo_hidden == PSQL_ECHO_HIDDEN_NOEXEC)
return NULL;
}
--
2.39.3 (Apple Git-146)
[application/octet-stream] v3-0001-Include-SQL-comments-on-echo-hidden-output.patch (14.6K, 3-v3-0001-Include-SQL-comments-on-echo-hidden-output.patch)
download | inline diff:
From 3f511da8c5fdc4a89ecc09a4d5b9b8eb76fee80f Mon Sep 17 00:00:00 2001
From: Greg Sabino Mullane <[email protected]>
Date: Thu, 21 Mar 2024 12:12:54 -0500
Subject: [PATCH v3 1/2] Include SQL comments on --echo-hidden output
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;
/************************/
---
src/bin/psql/describe.c | 82 ++++++++++++++++++++++++++---------------
1 file changed, 52 insertions(+), 30 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 68b2ea8872..7f272b2ac1 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1586,9 +1586,10 @@ describeOneTableDetails(const char *schemaname,
initPQExpBuffer(&tmpbuf);
/* Get general table info */
+ printfPQExpBuffer(&buf, _("/* Get general table information */\n"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"false AS relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1606,7 +1607,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1623,7 +1624,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, false as relispartition, %s, c.reltablespace, "
@@ -1640,7 +1641,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90400)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1657,7 +1658,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1718,9 +1719,10 @@ describeOneTableDetails(const char *schemaname,
printQueryOpt myopt = pset.popt;
char *footers[2] = {NULL, NULL};
+ printfPQExpBuffer(&buf, _("/* Get general sequence information */\n"));
if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
" seqstart AS \"%s\",\n"
" seqmin AS \"%s\",\n"
@@ -1744,7 +1746,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT 'bigint' AS \"%s\",\n"
" start_value AS \"%s\",\n"
" min_value AS \"%s\",\n"
@@ -1771,7 +1773,8 @@ describeOneTableDetails(const char *schemaname,
goto error_return;
/* Get the column that owns this sequence */
- printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
+ printfPQExpBuffer(&buf, _("/* Get the column that owns this sequence */\n"));
+ appendPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
"\n pg_catalog.quote_ident(relname) || '.' ||"
"\n pg_catalog.quote_ident(attname),"
"\n d.deptype"
@@ -1850,7 +1853,8 @@ describeOneTableDetails(const char *schemaname,
* duplicative test logic below.
*/
cols = 0;
- printfPQExpBuffer(&buf, "SELECT a.attname");
+ printfPQExpBuffer(&buf, _("/* Get information about each column */\n"));
+ appendPQExpBuffer(&buf, "SELECT a.attname");
attname_col = cols++;
appendPQExpBufferStr(&buf, ",\n pg_catalog.format_type(a.atttypid, a.atttypmod)");
atttype_col = cols++;
@@ -2150,7 +2154,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partition child table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get partition information for this table */\n"));
+ appendPQExpBuffer(&buf,
"SELECT inhparent::pg_catalog.regclass,\n"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid),\n ");
@@ -2205,7 +2210,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partitioned table (partitioning parent) */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get the partition key for this table */\n"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.pg_get_partkeydef('%s'::pg_catalog.oid);",
oid);
result = PSQLexec(buf.data);
@@ -2227,7 +2233,8 @@ describeOneTableDetails(const char *schemaname,
/* For a TOAST table, print name of owning table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Find which table owns this TOAST table */\n"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname, c.relname\n"
"FROM pg_catalog.pg_class c"
" JOIN pg_catalog.pg_namespace n"
@@ -2255,7 +2262,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information about an index */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get information about this index */\n"));
+ appendPQExpBuffer(&buf,
"SELECT i.indisunique, i.indisprimary, i.indisclustered, "
"i.indisvalid,\n"
" (NOT i.indimmediate) AND "
@@ -2372,7 +2380,8 @@ describeOneTableDetails(const char *schemaname,
/* print indexes */
if (tableinfo.hasindex)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get information about each index */\n"));
+ appendPQExpBuffer(&buf,
"SELECT c2.relname, i.indisprimary, i.indisunique, "
"i.indisclustered, i.indisvalid, "
"pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),\n "
@@ -2510,6 +2519,8 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ printfPQExpBuffer(&buf, _("/* Get information about foreign key constraints */\n"));
+
if (pset.sversion >= 120000 &&
(tableinfo.ispartition || tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
{
@@ -2517,7 +2528,7 @@ describeOneTableDetails(const char *schemaname,
* Put the constraints defined in this table first, followed
* by the constraints defined in ancestor partitioned tables.
*/
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conrelid = '%s'::pg_catalog.regclass AS sametable,\n"
" conname,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef,\n"
@@ -2530,7 +2541,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT true as sametable, conname,\n"
" pg_catalog.pg_get_constraintdef(r.oid, true) as condef,\n"
" conrelid::pg_catalog.regclass AS ontable\n"
@@ -2584,9 +2595,10 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ printfPQExpBuffer(&buf, _("/* Get information about incoming foreign key references */\n"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint c\n"
@@ -2598,7 +2610,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint\n"
@@ -2636,7 +2648,8 @@ describeOneTableDetails(const char *schemaname,
/* print any row-level policies */
if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf, "SELECT pol.polname,");
+ printfPQExpBuffer(&buf, _("/* Get information about row-level policies */\n"));
+ appendPQExpBuffer(&buf, "SELECT pol.polname,");
if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf,
" pol.polpermissive,\n");
@@ -2716,9 +2729,10 @@ describeOneTableDetails(const char *schemaname,
}
/* print any extended statistics */
+ printfPQExpBuffer(&buf, _("/* Get information about extended statistics */\n"));
if (pset.sversion >= 140000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace::pg_catalog.text AS nsp, "
@@ -2815,7 +2829,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace AS nsp, "
@@ -2894,7 +2908,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules && tableinfo.relkind != RELKIND_MATVIEW)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get information about each rule for this table */\n"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true)), "
"ev_enabled\n"
"FROM pg_catalog.pg_rewrite r\n"
@@ -2977,9 +2992,10 @@ describeOneTableDetails(const char *schemaname,
/* print any publications */
if (pset.sversion >= 100000)
{
+ printfPQExpBuffer(&buf, _("/* Get information about each publication using this table */\n"));
if (pset.sversion >= 150000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3011,7 +3027,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3061,7 +3077,9 @@ describeOneTableDetails(const char *schemaname,
/* If verbose, print NOT NULL constraints */
if (verbose)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get information about NOT NULL constraints */\n"));
+ appendPQExpBuffer(&buf,
+ "/* Find NOT NULL constraints */\n"
"SELECT co.conname, at.attname, co.connoinherit, co.conislocal,\n"
"co.coninhcount <> 0\n"
"FROM pg_catalog.pg_constraint co JOIN\n"
@@ -3133,7 +3151,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get information about each rule for this view */\n"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true))\n"
"FROM pg_catalog.pg_rewrite r\n"
"WHERE r.ev_class = '%s' AND r.rulename != '_RETURN' ORDER BY 1;",
@@ -3170,7 +3189,8 @@ describeOneTableDetails(const char *schemaname,
PGresult *result;
int tuples;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, _("/* Get information about each trigger on this table */\n"));
+ appendPQExpBuffer(&buf,
"SELECT t.tgname, "
"pg_catalog.pg_get_triggerdef(t.oid, true), "
"t.tgenabled, t.tgisinternal,\n");
@@ -3389,6 +3409,7 @@ describeOneTableDetails(const char *schemaname,
/* print tables inherited from (exclude partitioned parents) */
printfPQExpBuffer(&buf,
+ "/* Find tables inherited from */\n"
"SELECT c.oid::pg_catalog.regclass\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
"WHERE c.oid = i.inhparent AND i.inhrelid = '%s'\n"
@@ -3425,8 +3446,9 @@ describeOneTableDetails(const char *schemaname,
}
/* print child tables (with additional info if partitions) */
+ printfPQExpBuffer(&buf, _("/* Get information about child tables */\n"));
if (pset.sversion >= 140000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3436,7 +3458,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else if (pset.sversion >= 100000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3446,7 +3468,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending, NULL\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
--
2.39.3 (Apple Git-146)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2024-03-22 17:37 Greg Sabino Mullane <[email protected]>
parent: David Christensen <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Greg Sabino Mullane @ 2024-03-22 17:37 UTC (permalink / raw)
To: David Christensen <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
On Fri, Mar 22, 2024 at 11:39 AM David Christensen <[email protected]>
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
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2024-04-03 17:16 David Christensen <[email protected]>
parent: Greg Sabino Mullane <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David Christensen @ 2024-04-03 17:16 UTC (permalink / raw)
To: Greg Sabino Mullane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
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
Attachments:
[application/octet-stream] v4-0001-Improve-SQL-comments-on-echo-hidden-output.patch (21.1K, 2-v4-0001-Improve-SQL-comments-on-echo-hidden-output.patch)
download | inline diff:
From e92896202661f9f4d7707bbc33b934d4f2e2529a Mon Sep 17 00:00:00 2001
From: David Christensen <[email protected]>
Date: Wed, 3 Apr 2024 13:06:56 -0400
Subject: [PATCH v4] Improve SQL comments on --echo-hidden output
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!
For _all_ query output types (not just \d), we include the backslash command
that was used to generate the given query.
The new format looks like this:
/********************************* QUERY (\d) **********************************/
/* Get information about child tables */
SELECT c.oid::pg_catalog.regclass, c.relkind, inhdetachpending,
pg_catalog.pg_get_expr(c.relpartbound, c.oid)
FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
WHERE c.oid = i.inhrelid AND i.inhparent = '76992'
ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',
c.oid::pg_catalog.regclass::pg_catalog.text;
/*******************************************************************************/
This includes some level of refactoring the output routines in PSQLexec(),
adding OutputComment() and OutputCommentStars() to share a single output
width. (This could presumably be hoisted into something besides `psql`, but
since this is the only consumer here, leaving this for now.)
Since we are lining up all of the comments that are being output, we expand the
comment width to 80 chars.
Co-authored-by: Greg Sabino Mullane <[email protected]>
Co-authored-by: David Christensen <[email protected]>
---
src/bin/psql/command.c | 5 ++
src/bin/psql/common.c | 125 +++++++++++++++++++++++++++++++++++++---
src/bin/psql/common.h | 5 ++
src/bin/psql/describe.c | 81 ++++++++++++++++----------
4 files changed, 179 insertions(+), 37 deletions(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9b0fa041f7..4cfb91e134 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -56,6 +56,8 @@ typedef enum EditableObjectType
EditableView,
} EditableObjectType;
+char *curcmd = NULL;
+
/* local function declarations */
static backslashResult exec_command(const char *cmd,
PsqlScanState scan_state,
@@ -307,6 +309,7 @@ exec_command(const char *cmd,
cmd);
}
+ curcmd = (char *)cmd;
if (strcmp(cmd, "a") == 0)
status = exec_command_a(scan_state, active_branch);
else if (strcmp(cmd, "bind") == 0)
@@ -423,6 +426,8 @@ exec_command(const char *cmd,
else
status = PSQL_CMD_UNKNOWN;
+ curcmd = NULL;
+
/*
* All the commands that return PSQL_CMD_SEND want to execute previous_buf
* if query_buf is empty. For convenience we implement that here, not in
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 76e01b02a3..3e94bd5d12 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -42,6 +42,7 @@ static int ExecQueryAndProcessResults(const char *query,
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
+extern char *curcmd;
/*
* openQueryOutputFile --- attempt to open a query output file
@@ -581,6 +582,8 @@ PGresult *
PSQLexec(const char *query)
{
PGresult *res;
+ char *label = "QUERY";
+
if (!pset.db)
{
@@ -588,21 +591,32 @@ PSQLexec(const char *query)
return NULL;
}
+ if (curcmd)
+ label = psprintf("QUERY (\\%s)", curcmd);
+
if (pset.echo_hidden != PSQL_ECHO_HIDDEN_OFF)
{
- printf(_("/******** QUERY *********/\n"
- "%s\n"
- "/************************/\n\n"), query);
+ PQExpBufferData buf;
+ initPQExpBuffer(&buf);
+
+ OutputCommentStars(&buf, label);
+ appendPQExpBufferStr(&buf, query);
+ appendPQExpBufferChar(&buf, '\n');
+ OutputCommentStars(&buf, NULL);
+
+ printf("%s", buf.data);
fflush(stdout);
if (pset.logfile)
{
- fprintf(pset.logfile,
- _("/******** QUERY *********/\n"
- "%s\n"
- "/************************/\n\n"), query);
+ fprintf(pset.logfile, "%s", buf.data);
fflush(pset.logfile);
}
+ pfree(buf.data);
+
+ if (curcmd)
+ pfree(label);
+
if (pset.echo_hidden == PSQL_ECHO_HIDDEN_NOEXEC)
return NULL;
}
@@ -2434,3 +2448,100 @@ recognized_connection_string(const char *connstr)
{
return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
}
+
+/*
+ * OutputComment() outputs the given string to a buffer as a
+ * fixed width comment, wrapping the text given to the proper size and
+ * breaking at a space. We assume there are no inner comments that need to be
+ * escaped.
+ */
+void
+OutputComment(PQExpBufferData *buf, const char *string)
+{
+ int len = strlen(string);
+ int lineStart = 0;
+ int lineEnd = MAX_COMMENT_WIDTH - 6; // Accounting for " * " at the start and " *" at the end of each line
+ static char spaces[MAX_COMMENT_WIDTH] = {0};
+
+ if (spaces[0] != ' ')
+ memset(spaces, ' ', MAX_COMMENT_WIDTH);
+
+ while (lineStart < len) {
+ // Adjust lineEnd if it's beyond the length of the string
+ if (lineEnd >= len) {
+ lineEnd = len - 1;
+ } else {
+ // Ensure we break the line at a space (if possible) to avoid breaking words
+ while (lineEnd > lineStart && string[lineEnd] != ' ') {
+ lineEnd--;
+ }
+ if (lineEnd == lineStart) {
+ // If we couldn't find a space, force a break at the maximum line width
+ lineEnd = lineStart + MAX_COMMENT_WIDTH - 6;
+ }
+ }
+
+ // output our data for the width we have, including the piece of the comment
+ printfPQExpBuffer(buf,
+ "/* %.*s%.*s */\n",
+ lineEnd - lineStart + 1, /* number of chars to output */
+ string + lineStart, /* offset of chars to copy */
+ /* length of padding */
+ MAX_COMMENT_WIDTH - 6 - (lineEnd - lineStart),
+ spaces
+ );
+
+ // Move to the next line
+ lineStart = lineEnd + 1;
+ lineEnd = lineStart + MAX_COMMENT_WIDTH - 6;
+ }
+}
+
+
+/*
+ * OutputCommentStars() outputs the given single-line string wrapped in stars
+ * to the given width.
+ */
+void
+OutputCommentStars(PQExpBufferData *buf, const char *string)
+{
+ int len = string ? strlen(string) : 0;
+ char stars[MAX_COMMENT_WIDTH + 3];
+ int startOutputOffset;
+
+ /* This shouldn't happen based on current callers, but we'll truncate to a
+ * safe size if need be. */
+
+ if (len > MAX_COMMENT_WIDTH) /* space for opening comment, closing comment, and spaces */
+ len = MAX_COMMENT_WIDTH;
+
+ /* If we have a zero-width string then this is a special-case where we
+ * just want a line of stars. To minimize code disruption in this case,
+ * we'll just set startOutputOffset to a value larger than
+ * MAX_COMMENT_WIDTH. */
+
+ if (len > 0)
+ startOutputOffset = (((MAX_COMMENT_WIDTH - len - 2) / 2)); /* extra for space */
+ else
+ startOutputOffset = MAX_COMMENT_WIDTH * 2;
+
+ stars[0] = '/';
+ stars[MAX_COMMENT_WIDTH] = '/';
+ stars[MAX_COMMENT_WIDTH+1] = '\n';
+ stars[MAX_COMMENT_WIDTH+2] = '\0';
+
+ for (int i = 1; i < MAX_COMMENT_WIDTH; i++)
+ {
+ if (i >= startOutputOffset && i <= startOutputOffset + len + 1)
+ {
+ if (i == startOutputOffset || i == startOutputOffset + len + 1)
+ stars[i] = ' ';
+ else
+ stars[i] = string[i - startOutputOffset - 1];
+ }
+ else
+ stars[i] = '*';
+ }
+
+ appendPQExpBufferStr(buf, stars);
+}
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 6efe12274f..c16cbc6e6f 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -15,6 +15,8 @@
#include "fe_utils/psqlscan.h"
#include "libpq-fe.h"
+#define MAX_COMMENT_WIDTH 80
+
extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
extern bool setQFout(const char *fname);
@@ -44,4 +46,7 @@ extern void expand_tilde(char **filename);
extern bool recognized_connection_string(const char *connstr);
+extern void OutputComment(PQExpBufferData *buf, const char *string);
+extern void OutputCommentStars(PQExpBufferData *buf, const char *string);
+
#endif /* COMMON_H */
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 68b2ea8872..53b8fdd469 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1586,9 +1586,10 @@ describeOneTableDetails(const char *schemaname,
initPQExpBuffer(&tmpbuf);
/* Get general table info */
+ OutputComment(&buf, _("Get general table information"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"false AS relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1606,7 +1607,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1623,7 +1624,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, false as relispartition, %s, c.reltablespace, "
@@ -1640,7 +1641,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90400)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1657,7 +1658,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1718,9 +1719,10 @@ describeOneTableDetails(const char *schemaname,
printQueryOpt myopt = pset.popt;
char *footers[2] = {NULL, NULL};
+ OutputComment(&buf, _("Get general sequence information *"));
if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
" seqstart AS \"%s\",\n"
" seqmin AS \"%s\",\n"
@@ -1744,7 +1746,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT 'bigint' AS \"%s\",\n"
" start_value AS \"%s\",\n"
" min_value AS \"%s\",\n"
@@ -1771,7 +1773,8 @@ describeOneTableDetails(const char *schemaname,
goto error_return;
/* Get the column that owns this sequence */
- printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
+ OutputComment(&buf, _("Get the column that owns this sequence"));
+ appendPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
"\n pg_catalog.quote_ident(relname) || '.' ||"
"\n pg_catalog.quote_ident(attname),"
"\n d.deptype"
@@ -1850,7 +1853,8 @@ describeOneTableDetails(const char *schemaname,
* duplicative test logic below.
*/
cols = 0;
- printfPQExpBuffer(&buf, "SELECT a.attname");
+ OutputComment(&buf, _("Get information about each column\n"));
+ appendPQExpBuffer(&buf, "SELECT a.attname");
attname_col = cols++;
appendPQExpBufferStr(&buf, ",\n pg_catalog.format_type(a.atttypid, a.atttypmod)");
atttype_col = cols++;
@@ -2150,7 +2154,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partition child table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get partition information for this table"));
+ appendPQExpBuffer(&buf,
"SELECT inhparent::pg_catalog.regclass,\n"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid),\n ");
@@ -2205,7 +2210,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partitioned table (partitioning parent) */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get the partition key for this table"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.pg_get_partkeydef('%s'::pg_catalog.oid);",
oid);
result = PSQLexec(buf.data);
@@ -2227,7 +2233,8 @@ describeOneTableDetails(const char *schemaname,
/* For a TOAST table, print name of owning table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Find which table owns this TOAST table"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname, c.relname\n"
"FROM pg_catalog.pg_class c"
" JOIN pg_catalog.pg_namespace n"
@@ -2255,7 +2262,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information about an index */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about this index"));
+ appendPQExpBuffer(&buf,
"SELECT i.indisunique, i.indisprimary, i.indisclustered, "
"i.indisvalid,\n"
" (NOT i.indimmediate) AND "
@@ -2372,7 +2380,8 @@ describeOneTableDetails(const char *schemaname,
/* print indexes */
if (tableinfo.hasindex)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each index"));
+ appendPQExpBuffer(&buf,
"SELECT c2.relname, i.indisprimary, i.indisunique, "
"i.indisclustered, i.indisvalid, "
"pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),\n "
@@ -2510,6 +2519,8 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ OutputComment(&buf, _("Get information about foreign key constraints"));
+
if (pset.sversion >= 120000 &&
(tableinfo.ispartition || tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
{
@@ -2517,7 +2528,7 @@ describeOneTableDetails(const char *schemaname,
* Put the constraints defined in this table first, followed
* by the constraints defined in ancestor partitioned tables.
*/
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conrelid = '%s'::pg_catalog.regclass AS sametable,\n"
" conname,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef,\n"
@@ -2530,7 +2541,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT true as sametable, conname,\n"
" pg_catalog.pg_get_constraintdef(r.oid, true) as condef,\n"
" conrelid::pg_catalog.regclass AS ontable\n"
@@ -2584,9 +2595,10 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ OutputComment(&buf, _("Get information about incoming foreign key references"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint c\n"
@@ -2598,7 +2610,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint\n"
@@ -2636,7 +2648,8 @@ describeOneTableDetails(const char *schemaname,
/* print any row-level policies */
if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf, "SELECT pol.polname,");
+ OutputComment(&buf, _("Get information about row-level policies"));
+ appendPQExpBuffer(&buf, "SELECT pol.polname,");
if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf,
" pol.polpermissive,\n");
@@ -2716,9 +2729,10 @@ describeOneTableDetails(const char *schemaname,
}
/* print any extended statistics */
+ OutputComment(&buf, _("Get information about extended statistics"));
if (pset.sversion >= 140000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace::pg_catalog.text AS nsp, "
@@ -2815,7 +2829,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace AS nsp, "
@@ -2894,7 +2908,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules && tableinfo.relkind != RELKIND_MATVIEW)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each rule for this table"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true)), "
"ev_enabled\n"
"FROM pg_catalog.pg_rewrite r\n"
@@ -2977,9 +2992,10 @@ describeOneTableDetails(const char *schemaname,
/* print any publications */
if (pset.sversion >= 100000)
{
+ OutputComment(&buf, _("Get information about each publication using this table"));
if (pset.sversion >= 150000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3011,7 +3027,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3061,7 +3077,8 @@ describeOneTableDetails(const char *schemaname,
/* If verbose, print NOT NULL constraints */
if (verbose)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about NOT NULL constraints"));
+ appendPQExpBuffer(&buf,
"SELECT co.conname, at.attname, co.connoinherit, co.conislocal,\n"
"co.coninhcount <> 0\n"
"FROM pg_catalog.pg_constraint co JOIN\n"
@@ -3133,7 +3150,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each rule for this view"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true))\n"
"FROM pg_catalog.pg_rewrite r\n"
"WHERE r.ev_class = '%s' AND r.rulename != '_RETURN' ORDER BY 1;",
@@ -3170,7 +3188,8 @@ describeOneTableDetails(const char *schemaname,
PGresult *result;
int tuples;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each trigger on this table"));
+ appendPQExpBuffer(&buf,
"SELECT t.tgname, "
"pg_catalog.pg_get_triggerdef(t.oid, true), "
"t.tgenabled, t.tgisinternal,\n");
@@ -3388,6 +3407,7 @@ describeOneTableDetails(const char *schemaname,
}
/* print tables inherited from (exclude partitioned parents) */
+ OutputComment(&buf, _("Find tables inherited from"));
printfPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
@@ -3425,8 +3445,9 @@ describeOneTableDetails(const char *schemaname,
}
/* print child tables (with additional info if partitions) */
+ OutputComment(&buf, _("Get information about child tables"));
if (pset.sversion >= 140000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3436,7 +3457,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else if (pset.sversion >= 100000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3446,7 +3467,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending, NULL\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
--
2.40.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2024-04-04 14:31 Peter Eisentraut <[email protected]>
parent: David Christensen <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Peter Eisentraut @ 2024-04-04 14:31 UTC (permalink / raw)
To: David Christensen <[email protected]>; Greg Sabino Mullane <[email protected]>; +Cc: Jim Jones <[email protected]>; pgsql-hackers
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.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2024-04-04 16:12 David Christensen <[email protected]>
parent: Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David Christensen @ 2024-04-04 16:12 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
On Thu, Apr 4, 2024 at 9:32 AM Peter Eisentraut <[email protected]> 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
Attachments:
[application/octet-stream] v5-0001-Improve-SQL-comments-on-echo-hidden-output.patch (21.1K, 2-v5-0001-Improve-SQL-comments-on-echo-hidden-output.patch)
download | inline diff:
From f36f5edd50ac0fb64968b8ad784da09acc71c478 Mon Sep 17 00:00:00 2001
From: David Christensen <[email protected]>
Date: Thu, 4 Apr 2024 12:09:39 -0400
Subject: [PATCH v5] Improve SQL comments on --echo-hidden output
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!
For _all_ query output types (not just \d), we include the backslash command
that was used to generate the given query.
The new format looks like this:
/********************************* QUERY (\d) **********************************/
/* Get information about child tables */
SELECT c.oid::pg_catalog.regclass, c.relkind, inhdetachpending,
pg_catalog.pg_get_expr(c.relpartbound, c.oid)
FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i
WHERE c.oid = i.inhrelid AND i.inhparent = '76992'
ORDER BY pg_catalog.pg_get_expr(c.relpartbound, c.oid) = 'DEFAULT',
c.oid::pg_catalog.regclass::pg_catalog.text;
/*******************************************************************************/
This includes some level of refactoring the output routines in PSQLexec(),
adding OutputComment() and OutputCommentStars() to share a single output
width. (This could presumably be hoisted into something besides `psql`, but
since this is the only consumer here, leaving this for now.)
Since we are lining up all of the comments that are being output, we expand the
comment width to 80 chars.
Co-authored-by: Greg Sabino Mullane <[email protected]>
Co-authored-by: David Christensen <[email protected]>
---
src/bin/psql/command.c | 5 ++
src/bin/psql/common.c | 125 +++++++++++++++++++++++++++++++++++++---
src/bin/psql/common.h | 5 ++
src/bin/psql/describe.c | 81 ++++++++++++++++----------
4 files changed, 179 insertions(+), 37 deletions(-)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 9b0fa041f7..4cfb91e134 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -56,6 +56,8 @@ typedef enum EditableObjectType
EditableView,
} EditableObjectType;
+char *curcmd = NULL;
+
/* local function declarations */
static backslashResult exec_command(const char *cmd,
PsqlScanState scan_state,
@@ -307,6 +309,7 @@ exec_command(const char *cmd,
cmd);
}
+ curcmd = (char *)cmd;
if (strcmp(cmd, "a") == 0)
status = exec_command_a(scan_state, active_branch);
else if (strcmp(cmd, "bind") == 0)
@@ -423,6 +426,8 @@ exec_command(const char *cmd,
else
status = PSQL_CMD_UNKNOWN;
+ curcmd = NULL;
+
/*
* All the commands that return PSQL_CMD_SEND want to execute previous_buf
* if query_buf is empty. For convenience we implement that here, not in
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 76e01b02a3..5fd74ebc78 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -42,6 +42,7 @@ static int ExecQueryAndProcessResults(const char *query,
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
+extern char *curcmd;
/*
* openQueryOutputFile --- attempt to open a query output file
@@ -581,6 +582,8 @@ PGresult *
PSQLexec(const char *query)
{
PGresult *res;
+ char *label = _("QUERY");
+
if (!pset.db)
{
@@ -588,21 +591,32 @@ PSQLexec(const char *query)
return NULL;
}
+ if (curcmd)
+ label = psprintf(_("QUERY (\\%s)"), curcmd);
+
if (pset.echo_hidden != PSQL_ECHO_HIDDEN_OFF)
{
- printf(_("/******** QUERY *********/\n"
- "%s\n"
- "/************************/\n\n"), query);
+ PQExpBufferData buf;
+ initPQExpBuffer(&buf);
+
+ OutputCommentStars(&buf, label);
+ appendPQExpBufferStr(&buf, query);
+ appendPQExpBufferChar(&buf, '\n');
+ OutputCommentStars(&buf, NULL);
+
+ printf("%s", buf.data);
fflush(stdout);
if (pset.logfile)
{
- fprintf(pset.logfile,
- _("/******** QUERY *********/\n"
- "%s\n"
- "/************************/\n\n"), query);
+ fprintf(pset.logfile, "%s", buf.data);
fflush(pset.logfile);
}
+ pfree(buf.data);
+
+ if (curcmd)
+ pfree(label);
+
if (pset.echo_hidden == PSQL_ECHO_HIDDEN_NOEXEC)
return NULL;
}
@@ -2434,3 +2448,100 @@ recognized_connection_string(const char *connstr)
{
return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
}
+
+/*
+ * OutputComment() outputs the given string to a buffer as a
+ * fixed width comment, wrapping the text given to the proper size and
+ * breaking at a space. We assume there are no inner comments that need to be
+ * escaped.
+ */
+void
+OutputComment(PQExpBufferData *buf, const char *string)
+{
+ int len = strlen(string);
+ int lineStart = 0;
+ int lineEnd = MAX_COMMENT_WIDTH - 6; // Accounting for " * " at the start and " *" at the end of each line
+ static char spaces[MAX_COMMENT_WIDTH] = {0};
+
+ if (spaces[0] != ' ')
+ memset(spaces, ' ', MAX_COMMENT_WIDTH);
+
+ while (lineStart < len) {
+ // Adjust lineEnd if it's beyond the length of the string
+ if (lineEnd >= len) {
+ lineEnd = len - 1;
+ } else {
+ // Ensure we break the line at a space (if possible) to avoid breaking words
+ while (lineEnd > lineStart && string[lineEnd] != ' ') {
+ lineEnd--;
+ }
+ if (lineEnd == lineStart) {
+ // If we couldn't find a space, force a break at the maximum line width
+ lineEnd = lineStart + MAX_COMMENT_WIDTH - 6;
+ }
+ }
+
+ // output our data for the width we have, including the piece of the comment
+ printfPQExpBuffer(buf,
+ "/* %.*s%.*s */\n",
+ lineEnd - lineStart + 1, /* number of chars to output */
+ string + lineStart, /* offset of chars to copy */
+ /* length of padding */
+ MAX_COMMENT_WIDTH - 6 - (lineEnd - lineStart),
+ spaces
+ );
+
+ // Move to the next line
+ lineStart = lineEnd + 1;
+ lineEnd = lineStart + MAX_COMMENT_WIDTH - 6;
+ }
+}
+
+
+/*
+ * OutputCommentStars() outputs the given single-line string wrapped in stars
+ * to the given width.
+ */
+void
+OutputCommentStars(PQExpBufferData *buf, const char *string)
+{
+ int len = string ? strlen(string) : 0;
+ char stars[MAX_COMMENT_WIDTH + 3];
+ int startOutputOffset;
+
+ /* This shouldn't happen based on current callers, but we'll truncate to a
+ * safe size if need be. */
+
+ if (len > MAX_COMMENT_WIDTH) /* space for opening comment, closing comment, and spaces */
+ len = MAX_COMMENT_WIDTH;
+
+ /* If we have a zero-width string then this is a special-case where we
+ * just want a line of stars. To minimize code disruption in this case,
+ * we'll just set startOutputOffset to a value larger than
+ * MAX_COMMENT_WIDTH. */
+
+ if (len > 0)
+ startOutputOffset = (((MAX_COMMENT_WIDTH - len - 2) / 2)); /* extra for space */
+ else
+ startOutputOffset = MAX_COMMENT_WIDTH * 2;
+
+ stars[0] = '/';
+ stars[MAX_COMMENT_WIDTH] = '/';
+ stars[MAX_COMMENT_WIDTH+1] = '\n';
+ stars[MAX_COMMENT_WIDTH+2] = '\0';
+
+ for (int i = 1; i < MAX_COMMENT_WIDTH; i++)
+ {
+ if (i >= startOutputOffset && i <= startOutputOffset + len + 1)
+ {
+ if (i == startOutputOffset || i == startOutputOffset + len + 1)
+ stars[i] = ' ';
+ else
+ stars[i] = string[i - startOutputOffset - 1];
+ }
+ else
+ stars[i] = '*';
+ }
+
+ appendPQExpBufferStr(buf, stars);
+}
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 6efe12274f..c16cbc6e6f 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -15,6 +15,8 @@
#include "fe_utils/psqlscan.h"
#include "libpq-fe.h"
+#define MAX_COMMENT_WIDTH 80
+
extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
extern bool setQFout(const char *fname);
@@ -44,4 +46,7 @@ extern void expand_tilde(char **filename);
extern bool recognized_connection_string(const char *connstr);
+extern void OutputComment(PQExpBufferData *buf, const char *string);
+extern void OutputCommentStars(PQExpBufferData *buf, const char *string);
+
#endif /* COMMON_H */
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 68b2ea8872..53b8fdd469 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1586,9 +1586,10 @@ describeOneTableDetails(const char *schemaname,
initPQExpBuffer(&tmpbuf);
/* Get general table info */
+ OutputComment(&buf, _("Get general table information"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"false AS relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1606,7 +1607,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1623,7 +1624,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, false as relispartition, %s, c.reltablespace, "
@@ -1640,7 +1641,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90400)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1657,7 +1658,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1718,9 +1719,10 @@ describeOneTableDetails(const char *schemaname,
printQueryOpt myopt = pset.popt;
char *footers[2] = {NULL, NULL};
+ OutputComment(&buf, _("Get general sequence information *"));
if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
" seqstart AS \"%s\",\n"
" seqmin AS \"%s\",\n"
@@ -1744,7 +1746,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT 'bigint' AS \"%s\",\n"
" start_value AS \"%s\",\n"
" min_value AS \"%s\",\n"
@@ -1771,7 +1773,8 @@ describeOneTableDetails(const char *schemaname,
goto error_return;
/* Get the column that owns this sequence */
- printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
+ OutputComment(&buf, _("Get the column that owns this sequence"));
+ appendPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
"\n pg_catalog.quote_ident(relname) || '.' ||"
"\n pg_catalog.quote_ident(attname),"
"\n d.deptype"
@@ -1850,7 +1853,8 @@ describeOneTableDetails(const char *schemaname,
* duplicative test logic below.
*/
cols = 0;
- printfPQExpBuffer(&buf, "SELECT a.attname");
+ OutputComment(&buf, _("Get information about each column\n"));
+ appendPQExpBuffer(&buf, "SELECT a.attname");
attname_col = cols++;
appendPQExpBufferStr(&buf, ",\n pg_catalog.format_type(a.atttypid, a.atttypmod)");
atttype_col = cols++;
@@ -2150,7 +2154,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partition child table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get partition information for this table"));
+ appendPQExpBuffer(&buf,
"SELECT inhparent::pg_catalog.regclass,\n"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid),\n ");
@@ -2205,7 +2210,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partitioned table (partitioning parent) */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get the partition key for this table"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.pg_get_partkeydef('%s'::pg_catalog.oid);",
oid);
result = PSQLexec(buf.data);
@@ -2227,7 +2233,8 @@ describeOneTableDetails(const char *schemaname,
/* For a TOAST table, print name of owning table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Find which table owns this TOAST table"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname, c.relname\n"
"FROM pg_catalog.pg_class c"
" JOIN pg_catalog.pg_namespace n"
@@ -2255,7 +2262,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information about an index */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about this index"));
+ appendPQExpBuffer(&buf,
"SELECT i.indisunique, i.indisprimary, i.indisclustered, "
"i.indisvalid,\n"
" (NOT i.indimmediate) AND "
@@ -2372,7 +2380,8 @@ describeOneTableDetails(const char *schemaname,
/* print indexes */
if (tableinfo.hasindex)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each index"));
+ appendPQExpBuffer(&buf,
"SELECT c2.relname, i.indisprimary, i.indisunique, "
"i.indisclustered, i.indisvalid, "
"pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),\n "
@@ -2510,6 +2519,8 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ OutputComment(&buf, _("Get information about foreign key constraints"));
+
if (pset.sversion >= 120000 &&
(tableinfo.ispartition || tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
{
@@ -2517,7 +2528,7 @@ describeOneTableDetails(const char *schemaname,
* Put the constraints defined in this table first, followed
* by the constraints defined in ancestor partitioned tables.
*/
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conrelid = '%s'::pg_catalog.regclass AS sametable,\n"
" conname,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef,\n"
@@ -2530,7 +2541,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT true as sametable, conname,\n"
" pg_catalog.pg_get_constraintdef(r.oid, true) as condef,\n"
" conrelid::pg_catalog.regclass AS ontable\n"
@@ -2584,9 +2595,10 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ OutputComment(&buf, _("Get information about incoming foreign key references"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint c\n"
@@ -2598,7 +2610,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint\n"
@@ -2636,7 +2648,8 @@ describeOneTableDetails(const char *schemaname,
/* print any row-level policies */
if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf, "SELECT pol.polname,");
+ OutputComment(&buf, _("Get information about row-level policies"));
+ appendPQExpBuffer(&buf, "SELECT pol.polname,");
if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf,
" pol.polpermissive,\n");
@@ -2716,9 +2729,10 @@ describeOneTableDetails(const char *schemaname,
}
/* print any extended statistics */
+ OutputComment(&buf, _("Get information about extended statistics"));
if (pset.sversion >= 140000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace::pg_catalog.text AS nsp, "
@@ -2815,7 +2829,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace AS nsp, "
@@ -2894,7 +2908,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules && tableinfo.relkind != RELKIND_MATVIEW)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each rule for this table"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true)), "
"ev_enabled\n"
"FROM pg_catalog.pg_rewrite r\n"
@@ -2977,9 +2992,10 @@ describeOneTableDetails(const char *schemaname,
/* print any publications */
if (pset.sversion >= 100000)
{
+ OutputComment(&buf, _("Get information about each publication using this table"));
if (pset.sversion >= 150000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3011,7 +3027,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3061,7 +3077,8 @@ describeOneTableDetails(const char *schemaname,
/* If verbose, print NOT NULL constraints */
if (verbose)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about NOT NULL constraints"));
+ appendPQExpBuffer(&buf,
"SELECT co.conname, at.attname, co.connoinherit, co.conislocal,\n"
"co.coninhcount <> 0\n"
"FROM pg_catalog.pg_constraint co JOIN\n"
@@ -3133,7 +3150,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each rule for this view"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true))\n"
"FROM pg_catalog.pg_rewrite r\n"
"WHERE r.ev_class = '%s' AND r.rulename != '_RETURN' ORDER BY 1;",
@@ -3170,7 +3188,8 @@ describeOneTableDetails(const char *schemaname,
PGresult *result;
int tuples;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each trigger on this table"));
+ appendPQExpBuffer(&buf,
"SELECT t.tgname, "
"pg_catalog.pg_get_triggerdef(t.oid, true), "
"t.tgenabled, t.tgisinternal,\n");
@@ -3388,6 +3407,7 @@ describeOneTableDetails(const char *schemaname,
}
/* print tables inherited from (exclude partitioned parents) */
+ OutputComment(&buf, _("Find tables inherited from"));
printfPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
@@ -3425,8 +3445,9 @@ describeOneTableDetails(const char *schemaname,
}
/* print child tables (with additional info if partitions) */
+ OutputComment(&buf, _("Get information about child tables"));
if (pset.sversion >= 140000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3436,7 +3457,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else if (pset.sversion >= 100000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3446,7 +3467,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending, NULL\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
--
2.40.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2024-06-11 22:54 David Christensen <[email protected]>
parent: David Christensen <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David Christensen @ 2024-06-11 22:54 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Greg Sabino Mullane <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
On Thu, Apr 4, 2024 at 11:12 AM David Christensen <[email protected]> wrote:
>
> On Thu, Apr 4, 2024 at 9:32 AM Peter Eisentraut <[email protected]> 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
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2025-01-18 20:37 Tom Lane <[email protected]>
parent: David Christensen <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2025-01-18 20:37 UTC (permalink / raw)
To: David Christensen <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Greg Sabino Mullane <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
David Christensen <[email protected]> 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
Attachments:
[text/x-diff] v6-0001-Improve-SQL-comments-on-echo-hidden-output.patch (18.6K, 2-v6-0001-Improve-SQL-comments-on-echo-hidden-output.patch)
download | inline diff:
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 613583145e..f5b8c1d369 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -53,6 +53,8 @@ typedef enum EditableObjectType
EditableView,
} EditableObjectType;
+char *curcmd = NULL;
+
/* local function declarations */
static backslashResult exec_command(const char *cmd,
PsqlScanState scan_state,
@@ -311,6 +313,7 @@ exec_command(const char *cmd,
cmd);
}
+ curcmd = (char *) cmd;
if (strcmp(cmd, "a") == 0)
status = exec_command_a(scan_state, active_branch);
else if (strcmp(cmd, "bind") == 0)
@@ -438,6 +441,8 @@ exec_command(const char *cmd,
else
status = PSQL_CMD_UNKNOWN;
+ curcmd = NULL;
+
/*
* All the commands that return PSQL_CMD_SEND want to execute previous_buf
* if query_buf is empty. For convenience we implement that here, not in
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index f1a5291c13..d5f7fa9e64 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -40,6 +40,7 @@ static int ExecQueryAndProcessResults(const char *query,
FILE *printQueryFout);
static bool command_no_begin(const char *query);
+extern char *curcmd;
/*
* openQueryOutputFile --- attempt to open a query output file
@@ -620,6 +621,8 @@ PGresult *
PSQLexec(const char *query)
{
PGresult *res;
+ char *label = _("QUERY");
+
if (!pset.db)
{
@@ -627,21 +630,33 @@ PSQLexec(const char *query)
return NULL;
}
+ if (curcmd)
+ label = psprintf(_("QUERY (\\%s)"), curcmd);
+
if (pset.echo_hidden != PSQL_ECHO_HIDDEN_OFF)
{
- printf(_("/******** QUERY *********/\n"
- "%s\n"
- "/************************/\n\n"), query);
+ PQExpBufferData buf;
+
+ initPQExpBuffer(&buf);
+
+ OutputCommentStars(&buf, label);
+ appendPQExpBufferStr(&buf, query);
+ appendPQExpBufferChar(&buf, '\n');
+ OutputCommentStars(&buf, NULL);
+
+ printf("%s", buf.data);
fflush(stdout);
if (pset.logfile)
{
- fprintf(pset.logfile,
- _("/******** QUERY *********/\n"
- "%s\n"
- "/************************/\n\n"), query);
+ fprintf(pset.logfile, "%s", buf.data);
fflush(pset.logfile);
}
+ pfree(buf.data);
+
+ if (curcmd)
+ pfree(label);
+
if (pset.echo_hidden == PSQL_ECHO_HIDDEN_NOEXEC)
return NULL;
}
@@ -2318,3 +2333,123 @@ recognized_connection_string(const char *connstr)
{
return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
}
+
+/*
+ * OutputComment() outputs the given string to a buffer as a
+ * fixed width comment, wrapping the text given to the proper size and
+ * breaking at a space. We assume there are no inner comments that need to be
+ * escaped.
+ */
+void
+OutputComment(PQExpBufferData *buf, const char *string)
+{
+ int len = strlen(string);
+ int lineStart = 0;
+
+ /* -6 accounts for " * " at the start and " *" at the end of each line */
+ int lineEnd = MAX_COMMENT_WIDTH - 6;
+ static char spaces[MAX_COMMENT_WIDTH] = {0};
+
+ if (spaces[0] != ' ')
+ memset(spaces, ' ', MAX_COMMENT_WIDTH);
+
+ while (lineStart < len)
+ {
+ /* Adjust lineEnd if it's beyond the length of the string */
+ if (lineEnd >= len)
+ {
+ lineEnd = len - 1;
+ }
+ else
+ {
+ /*
+ * Ensure we break the line at a space (if possible) to avoid
+ * breaking words
+ */
+ while (lineEnd > lineStart && string[lineEnd] != ' ')
+ {
+ lineEnd--;
+ }
+ if (lineEnd == lineStart)
+ {
+ /*
+ * If we couldn't find a space, force a break at the maximum
+ * line width
+ */
+ lineEnd = lineStart + MAX_COMMENT_WIDTH - 6;
+ }
+ }
+
+ /*
+ * output our data for the width we have, including the piece of the
+ * comment
+ */
+ printfPQExpBuffer(buf,
+ "/* %.*s%.*s */\n",
+ lineEnd - lineStart + 1, /* number of chars to
+ * output */
+ string + lineStart, /* offset of chars to copy */
+ /* length of padding */
+ MAX_COMMENT_WIDTH - 6 - (lineEnd - lineStart),
+ spaces
+ );
+
+ /* Move to the next line */
+ lineStart = lineEnd + 1;
+ lineEnd = lineStart + MAX_COMMENT_WIDTH - 6;
+ }
+}
+
+
+/*
+ * OutputCommentStars() outputs the given single-line string wrapped in stars
+ * to the given width.
+ */
+void
+OutputCommentStars(PQExpBufferData *buf, const char *string)
+{
+ int len = string ? strlen(string) : 0;
+ char stars[MAX_COMMENT_WIDTH + 3];
+ int startOutputOffset;
+
+ /*
+ * This shouldn't happen based on current callers, but we'll truncate to a
+ * safe size if need be.
+ */
+
+ if (len > MAX_COMMENT_WIDTH) /* space for opening comment, closing
+ * comment, and spaces */
+ len = MAX_COMMENT_WIDTH;
+
+ /*
+ * If we have a zero-width string then this is a special-case where we
+ * just want a line of stars. To minimize code disruption in this case,
+ * we'll just set startOutputOffset to a value larger than
+ * MAX_COMMENT_WIDTH.
+ */
+
+ if (len > 0)
+ startOutputOffset = (((MAX_COMMENT_WIDTH - len - 2) / 2)); /* extra for space */
+ else
+ startOutputOffset = MAX_COMMENT_WIDTH * 2;
+
+ stars[0] = '/';
+ stars[MAX_COMMENT_WIDTH] = '/';
+ stars[MAX_COMMENT_WIDTH + 1] = '\n';
+ stars[MAX_COMMENT_WIDTH + 2] = '\0';
+
+ for (int i = 1; i < MAX_COMMENT_WIDTH; i++)
+ {
+ if (i >= startOutputOffset && i <= startOutputOffset + len + 1)
+ {
+ if (i == startOutputOffset || i == startOutputOffset + len + 1)
+ stars[i] = ' ';
+ else
+ stars[i] = string[i - startOutputOffset - 1];
+ }
+ else
+ stars[i] = '*';
+ }
+
+ appendPQExpBufferStr(buf, stars);
+}
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 7f1a23de1e..5d1c3d63ab 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -15,6 +15,8 @@
#include "fe_utils/psqlscan.h"
#include "libpq-fe.h"
+#define MAX_COMMENT_WIDTH 80
+
extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
extern bool setQFout(const char *fname);
@@ -45,4 +47,7 @@ extern void clean_extended_state(void);
extern bool recognized_connection_string(const char *connstr);
+extern void OutputComment(PQExpBufferData *buf, const char *string);
+extern void OutputCommentStars(PQExpBufferData *buf, const char *string);
+
#endif /* COMMON_H */
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 2ef99971ac..af3cd40543 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1623,9 +1623,10 @@ describeOneTableDetails(const char *schemaname,
initPQExpBuffer(&tmpbuf);
/* Get general table info */
+ OutputComment(&buf, _("Get general table information"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"false AS relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1643,7 +1644,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1660,7 +1661,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, false as relispartition, %s, c.reltablespace, "
@@ -1677,7 +1678,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90400)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1694,7 +1695,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1755,9 +1756,10 @@ describeOneTableDetails(const char *schemaname,
printQueryOpt myopt = pset.popt;
char *footers[2] = {NULL, NULL};
+ OutputComment(&buf, _("Get general sequence information *"));
if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
" seqstart AS \"%s\",\n"
" seqmin AS \"%s\",\n"
@@ -1781,7 +1783,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT 'bigint' AS \"%s\",\n"
" start_value AS \"%s\",\n"
" min_value AS \"%s\",\n"
@@ -1808,7 +1810,8 @@ describeOneTableDetails(const char *schemaname,
goto error_return;
/* Get the column that owns this sequence */
- printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
+ OutputComment(&buf, _("Get the column that owns this sequence"));
+ appendPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
"\n pg_catalog.quote_ident(relname) || '.' ||"
"\n pg_catalog.quote_ident(attname),"
"\n d.deptype"
@@ -1887,7 +1890,8 @@ describeOneTableDetails(const char *schemaname,
* duplicative test logic below.
*/
cols = 0;
- printfPQExpBuffer(&buf, "SELECT a.attname");
+ OutputComment(&buf, _("Get information about each column\n"));
+ appendPQExpBuffer(&buf, "SELECT a.attname");
attname_col = cols++;
appendPQExpBufferStr(&buf, ",\n pg_catalog.format_type(a.atttypid, a.atttypmod)");
atttype_col = cols++;
@@ -2183,7 +2187,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partition child table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get partition information for this table"));
+ appendPQExpBuffer(&buf,
"SELECT inhparent::pg_catalog.regclass,\n"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid),\n ");
@@ -2238,7 +2243,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partitioned table (partitioning parent) */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get the partition key for this table"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.pg_get_partkeydef('%s'::pg_catalog.oid);",
oid);
result = PSQLexec(buf.data);
@@ -2260,7 +2266,8 @@ describeOneTableDetails(const char *schemaname,
/* For a TOAST table, print name of owning table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Find which table owns this TOAST table"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname, c.relname\n"
"FROM pg_catalog.pg_class c"
" JOIN pg_catalog.pg_namespace n"
@@ -2288,7 +2295,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information about an index */
PGresult *result;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about this index"));
+ appendPQExpBuffer(&buf,
"SELECT i.indisunique, i.indisprimary, i.indisclustered, "
"i.indisvalid,\n"
" (NOT i.indimmediate) AND "
@@ -2409,7 +2417,8 @@ describeOneTableDetails(const char *schemaname,
/* print indexes */
if (tableinfo.hasindex)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each index"));
+ appendPQExpBuffer(&buf,
"SELECT c2.relname, i.indisprimary, i.indisunique, "
"i.indisclustered, i.indisvalid, "
"pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),\n "
@@ -2551,6 +2560,8 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ OutputComment(&buf, _("Get information about foreign key constraints"));
+
if (pset.sversion >= 120000 &&
(tableinfo.ispartition || tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
{
@@ -2558,7 +2569,7 @@ describeOneTableDetails(const char *schemaname,
* Put the constraints defined in this table first, followed
* by the constraints defined in ancestor partitioned tables.
*/
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conrelid = '%s'::pg_catalog.regclass AS sametable,\n"
" conname,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef,\n"
@@ -2571,7 +2582,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT true as sametable, conname,\n"
" pg_catalog.pg_get_constraintdef(r.oid, true) as condef,\n"
" conrelid::pg_catalog.regclass AS ontable\n"
@@ -2625,9 +2636,10 @@ describeOneTableDetails(const char *schemaname,
if (tableinfo.hastriggers ||
tableinfo.relkind == RELKIND_PARTITIONED_TABLE)
{
+ OutputComment(&buf, _("Get information about incoming foreign key references"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint c\n"
@@ -2639,7 +2651,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint\n"
@@ -2677,7 +2689,8 @@ describeOneTableDetails(const char *schemaname,
/* print any row-level policies */
if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf, "SELECT pol.polname,");
+ OutputComment(&buf, _("Get information about row-level policies"));
+ appendPQExpBuffer(&buf, "SELECT pol.polname,");
if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf,
" pol.polpermissive,\n");
@@ -2757,9 +2770,10 @@ describeOneTableDetails(const char *schemaname,
}
/* print any extended statistics */
+ OutputComment(&buf, _("Get information about extended statistics"));
if (pset.sversion >= 140000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace::pg_catalog.text AS nsp, "
@@ -2857,7 +2871,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace AS nsp, "
@@ -2936,7 +2950,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules && tableinfo.relkind != RELKIND_MATVIEW)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each rule for this table"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true)), "
"ev_enabled\n"
"FROM pg_catalog.pg_rewrite r\n"
@@ -3019,9 +3034,10 @@ describeOneTableDetails(const char *schemaname,
/* print any publications */
if (pset.sversion >= 100000)
{
+ OutputComment(&buf, _("Get information about each publication using this table"));
if (pset.sversion >= 150000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3053,7 +3069,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3105,7 +3121,8 @@ describeOneTableDetails(const char *schemaname,
*/
if (verbose)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about NOT NULL constraints"));
+ appendPQExpBuffer(&buf,
"SELECT c.conname, a.attname, c.connoinherit,\n"
" c.conislocal, c.coninhcount <> 0\n"
"FROM pg_catalog.pg_constraint c JOIN\n"
@@ -3175,7 +3192,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules)
{
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each rule for this view"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true))\n"
"FROM pg_catalog.pg_rewrite r\n"
"WHERE r.ev_class = '%s' AND r.rulename != '_RETURN' ORDER BY 1;",
@@ -3212,7 +3230,8 @@ describeOneTableDetails(const char *schemaname,
PGresult *result;
int tuples;
- printfPQExpBuffer(&buf,
+ OutputComment(&buf, _("Get information about each trigger on this table"));
+ appendPQExpBuffer(&buf,
"SELECT t.tgname, "
"pg_catalog.pg_get_triggerdef(t.oid, true), "
"t.tgenabled, t.tgisinternal,\n");
@@ -3430,6 +3449,7 @@ describeOneTableDetails(const char *schemaname,
}
/* print tables inherited from (exclude partitioned parents) */
+ OutputComment(&buf, _("Find tables inherited from"));
printfPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
@@ -3467,8 +3487,9 @@ describeOneTableDetails(const char *schemaname,
}
/* print child tables (with additional info if partitions) */
+ OutputComment(&buf, _("Get information about child tables"));
if (pset.sversion >= 140000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3478,7 +3499,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else if (pset.sversion >= 100000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3488,7 +3509,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending, NULL\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2025-01-30 20:55 David Christensen <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: David Christensen @ 2025-01-30 20:55 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Greg Sabino Mullane <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
On Sat, Jan 18, 2025 at 2:37 PM Tom Lane <[email protected]> wrote:
>
> David Christensen <[email protected]> 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
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2025-04-01 00:02 Maiquel Grassi <[email protected]>
parent: David Christensen <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Maiquel Grassi @ 2025-04-01 00:02 UTC (permalink / raw)
To: David Christensen <[email protected]>; Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Greg Sabino Mullane <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
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.
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2026-03-22 15:15 Greg Sabino Mullane <[email protected]>
parent: Maiquel Grassi <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Greg Sabino Mullane @ 2026-03-22 15:15 UTC (permalink / raw)
To: Maiquel Grassi <[email protected]>; +Cc: David Christensen <[email protected]>; Tom Lane <[email protected]>; Peter Eisentraut <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
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
Attachments:
[application/octet-stream] 0007-Add-comment-header-for-generated-SQL-inside-psql.patch (34.7K, 3-0007-Add-comment-header-for-generated-SQL-inside-psql.patch)
download | inline diff:
From 8e9cc8b7def3614f0f643f0811bc56b99d9ffe3d Mon Sep 17 00:00:00 2001
From: Greg Sabino Mullane <[email protected]>
Date: Sun, 22 Mar 2026 11:06:01 -0400
Subject: [PATCH] Add comment header for generated SQL inside psql
---
src/bin/psql/describe.c | 265 ++++++++++++++++++++++++++--------------
1 file changed, 176 insertions(+), 89 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index eafb33143d9..f09cd49850c 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -84,7 +84,8 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching aggregates"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" p.proname AS \"%s\",\n"
" pg_catalog.format_type(p.prorettype, NULL) AS \"%s\",\n"
@@ -165,7 +166,8 @@ describeAccessMethods(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching access methods"));
+ appendPQExpBuffer(&buf,
"SELECT amname AS \"%s\",\n"
" CASE amtype"
" WHEN " CppAsString2(AMTYPE_INDEX) " THEN '%s'"
@@ -228,7 +230,8 @@ describeTablespaces(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching tablespaces"));
+ appendPQExpBuffer(&buf,
"SELECT spcname AS \"%s\",\n"
" pg_catalog.pg_get_userbyid(spcowner) AS \"%s\",\n"
" pg_catalog.pg_tablespace_location(oid) AS \"%s\"",
@@ -338,7 +341,8 @@ describeFunctions(const char *functypes, const char *func_pattern,
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching functions"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" p.proname as \"%s\",\n",
gettext_noop("Schema"),
@@ -645,7 +649,8 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching types"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" pg_catalog.format_type(t.oid, NULL) AS \"%s\",\n",
gettext_noop("Schema"),
@@ -819,7 +824,8 @@ describeOperators(const char *oper_pattern,
* to pre-v14 servers.
*/
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching operators"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" o.oprname AS \"%s\",\n"
" CASE WHEN o.oprkind='l' THEN NULL ELSE pg_catalog.format_type(o.oprleft, NULL) END AS \"%s\",\n"
@@ -952,7 +958,8 @@ listAllDbs(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching databases"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" d.datname as \"%s\",\n"
" pg_catalog.pg_get_userbyid(d.datdba) as \"%s\",\n"
@@ -1060,7 +1067,8 @@ permissionsList(const char *pattern, bool showSystem)
/*
* we ignore indexes and toast tables since they have no meaningful rights
*/
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching access privileges"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" c.relname as \"%s\",\n"
" CASE c.relkind"
@@ -1224,7 +1232,8 @@ listDefaultACLs(const char *pattern)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching default ACLs"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.pg_get_userbyid(d.defaclrole) AS \"%s\",\n"
" n.nspname AS \"%s\",\n"
" CASE d.defaclobjtype "
@@ -1305,6 +1314,7 @@ objectDescription(const char *pattern, bool showSystem)
initPQExpBuffer(&buf);
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching object comments"));
appendPQExpBuffer(&buf,
"SELECT DISTINCT tt.nspname AS \"%s\", tt.name AS \"%s\", tt.object AS \"%s\", d.description AS \"%s\"\n"
"FROM (\n",
@@ -1497,7 +1507,8 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching relations to describe"));
+ appendPQExpBuffer(&buf,
"SELECT c.oid,\n"
" n.nspname,\n"
" c.relname\n"
@@ -1633,9 +1644,10 @@ describeOneTableDetails(const char *schemaname,
initPQExpBuffer(&tmpbuf);
/* Get general table info */
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get general relation information"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"false AS relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1653,7 +1665,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, c.relispartition, %s, c.reltablespace, "
@@ -1670,7 +1682,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, c.relrowsecurity, c.relforcerowsecurity, "
"c.relhasoids, false as relispartition, %s, c.reltablespace, "
@@ -1687,7 +1699,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 90400)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1704,7 +1716,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.relchecks, c.relkind, c.relhasindex, c.relhasrules, "
"c.relhastriggers, false, false, c.relhasoids, "
"false as relispartition, %s, c.reltablespace, "
@@ -1765,9 +1777,10 @@ describeOneTableDetails(const char *schemaname,
printQueryOpt myopt = pset.popt;
char *footers[3] = {NULL, NULL, NULL};
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get sequence information"));
if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(seqtypid, NULL) AS \"%s\",\n"
" seqstart AS \"%s\",\n"
" seqmin AS \"%s\",\n"
@@ -1791,7 +1804,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT 'bigint' AS \"%s\",\n"
" start_value AS \"%s\",\n"
" min_value AS \"%s\",\n"
@@ -1818,7 +1831,8 @@ describeOneTableDetails(const char *schemaname,
goto error_return;
/* Get the column that owns this sequence */
- printfPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get the column that owns this sequence"));
+ appendPQExpBuffer(&buf, "SELECT pg_catalog.quote_ident(nspname) || '.' ||"
"\n pg_catalog.quote_ident(relname) || '.' ||"
"\n pg_catalog.quote_ident(attname),"
"\n d.deptype"
@@ -1862,7 +1876,8 @@ describeOneTableDetails(const char *schemaname,
/* Print any publications */
if (pset.sversion >= 190000)
{
- printfPQExpBuffer(&buf, "SELECT pubname FROM pg_catalog.pg_publication p"
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get publications containing this sequence"));
+ appendPQExpBuffer(&buf, "SELECT pubname FROM pg_catalog.pg_publication p"
"\nWHERE p.puballsequences"
"\n AND pg_catalog.pg_relation_is_publishable('%s')"
"\nORDER BY 1",
@@ -1921,7 +1936,8 @@ describeOneTableDetails(const char *schemaname,
printQueryOpt myopt = pset.popt;
char *footers[3] = {NULL, NULL, NULL};
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get property graph information"));
+ appendPQExpBuffer(&buf,
"SELECT e.pgealias AS \"%s\","
"\n pg_catalog.quote_ident(n.nspname) || '.' ||"
"\n pg_catalog.quote_ident(c.relname) AS \"%s\","
@@ -2003,7 +2019,8 @@ describeOneTableDetails(const char *schemaname,
* duplicative test logic below.
*/
cols = 0;
- printfPQExpBuffer(&buf, "SELECT a.attname");
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get columns for this relation"));
+ appendPQExpBuffer(&buf, "SELECT a.attname");
attname_col = cols++;
appendPQExpBufferStr(&buf, ",\n pg_catalog.format_type(a.atttypid, a.atttypmod)");
atttype_col = cols++;
@@ -2305,7 +2322,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information for a partition child table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get partition information for this table"));
+ appendPQExpBuffer(&buf,
"SELECT inhparent::pg_catalog.regclass,\n"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid),\n ");
@@ -2382,7 +2400,8 @@ describeOneTableDetails(const char *schemaname,
/* For a TOAST table, print name of owning table */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get the table which owns this TOAST table"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname, c.relname\n"
"FROM pg_catalog.pg_class c"
" JOIN pg_catalog.pg_namespace n"
@@ -2410,7 +2429,8 @@ describeOneTableDetails(const char *schemaname,
/* Footer information about an index */
PGresult *result;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get index details"));
+ appendPQExpBuffer(&buf,
"SELECT i.indisunique, i.indisprimary, i.indisclustered, "
"i.indisvalid,\n"
" (NOT i.indimmediate) AND "
@@ -2531,7 +2551,8 @@ describeOneTableDetails(const char *schemaname,
/* print indexes */
if (tableinfo.hasindex)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get indexes for this table"));
+ appendPQExpBuffer(&buf,
"SELECT c2.relname, i.indisprimary, i.indisunique, "
"i.indisclustered, i.indisvalid, "
"pg_catalog.pg_get_indexdef(i.indexrelid, 0, true),\n "
@@ -2635,7 +2656,9 @@ describeOneTableDetails(const char *schemaname,
/* print table (and column) check constraints */
if (tableinfo.checks)
{
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get check constraints"));
+ appendPQExpBuffer(&buf,
"SELECT r.conname, "
"pg_catalog.pg_get_constraintdef(r.oid, true)\n"
"FROM pg_catalog.pg_constraint r\n"
@@ -2666,6 +2689,7 @@ describeOneTableDetails(const char *schemaname,
}
/* Print foreign-key constraints */
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get foreign key constraints"));
if (pset.sversion >= 120000 &&
(tableinfo.ispartition || tableinfo.relkind == RELKIND_PARTITIONED_TABLE))
{
@@ -2673,7 +2697,7 @@ describeOneTableDetails(const char *schemaname,
* Put the constraints defined in this table first, followed by
* the constraints defined in ancestor partitioned tables.
*/
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conrelid = '%s'::pg_catalog.regclass AS sametable,\n"
" conname,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef,\n"
@@ -2686,7 +2710,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT true as sametable, conname,\n"
" pg_catalog.pg_get_constraintdef(r.oid, true) as condef,\n"
" conrelid::pg_catalog.regclass AS ontable\n"
@@ -2736,9 +2760,10 @@ describeOneTableDetails(const char *schemaname,
PQclear(result);
/* print incoming foreign-key references */
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get foreign keys referencing this table"));
if (pset.sversion >= 120000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint c\n"
@@ -2750,7 +2775,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT conname, conrelid::pg_catalog.regclass AS ontable,\n"
" pg_catalog.pg_get_constraintdef(oid, true) AS condef\n"
" FROM pg_catalog.pg_constraint\n"
@@ -2787,7 +2812,8 @@ describeOneTableDetails(const char *schemaname,
/* print any row-level policies */
if (pset.sversion >= 90500)
{
- printfPQExpBuffer(&buf, "SELECT pol.polname,");
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get row-level policies for this table"));
+ appendPQExpBuffer(&buf, "SELECT pol.polname,");
if (pset.sversion >= 100000)
appendPQExpBufferStr(&buf,
" pol.polpermissive,\n");
@@ -2867,9 +2893,10 @@ describeOneTableDetails(const char *schemaname,
}
/* print any extended statistics */
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get extended statistics for this table"));
if (pset.sversion >= 140000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace::pg_catalog.text AS nsp, "
@@ -2967,7 +2994,7 @@ describeOneTableDetails(const char *schemaname,
}
else if (pset.sversion >= 100000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT oid, "
"stxrelid::pg_catalog.regclass, "
"stxnamespace::pg_catalog.regnamespace AS nsp, "
@@ -3046,7 +3073,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules && tableinfo.relkind != RELKIND_MATVIEW)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get rules for this table"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true)), "
"ev_enabled\n"
"FROM pg_catalog.pg_rewrite r\n"
@@ -3129,9 +3157,10 @@ describeOneTableDetails(const char *schemaname,
/* print any publications */
if (pset.sversion >= 100000)
{
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get publications that use this table"));
if (pset.sversion >= 150000)
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3191,7 +3220,7 @@ describeOneTableDetails(const char *schemaname,
}
else
{
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
" , NULL\n"
" , NULL\n"
@@ -3241,7 +3270,8 @@ describeOneTableDetails(const char *schemaname,
/* Print publications where the table is in the EXCEPT clause */
if (pset.sversion >= 190000)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get publications that exclude this table"));
+ appendPQExpBuffer(&buf,
"SELECT pubname\n"
"FROM pg_catalog.pg_publication p\n"
"JOIN pg_catalog.pg_publication_rel pr ON p.oid = pr.prpubid\n"
@@ -3273,7 +3303,8 @@ describeOneTableDetails(const char *schemaname,
*/
if (verbose)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get not null constraints for this table"));
+ appendPQExpBuffer(&buf,
"SELECT c.conname, a.attname, c.connoinherit,\n"
" c.conislocal, c.coninhcount <> 0,\n"
" c.convalidated\n"
@@ -3346,7 +3377,8 @@ describeOneTableDetails(const char *schemaname,
/* print rules */
if (tableinfo.hasrules)
{
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get rules for this view"));
+ appendPQExpBuffer(&buf,
"SELECT r.rulename, trim(trailing ';' from pg_catalog.pg_get_ruledef(r.oid, true))\n"
"FROM pg_catalog.pg_rewrite r\n"
"WHERE r.ev_class = '%s' AND r.rulename != '_RETURN' ORDER BY 1;",
@@ -3383,7 +3415,8 @@ describeOneTableDetails(const char *schemaname,
PGresult *result;
int tuples;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get triggers for this table"));
+ appendPQExpBuffer(&buf,
"SELECT t.tgname, "
"pg_catalog.pg_get_triggerdef(t.oid, true), "
"t.tgenabled, t.tgisinternal,\n");
@@ -3566,7 +3599,8 @@ describeOneTableDetails(const char *schemaname,
char *ftoptions;
/* Footer information about foreign table */
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get foreign server"));
+ appendPQExpBuffer(&buf,
"SELECT s.srvname,\n"
" pg_catalog.array_to_string(ARRAY(\n"
" SELECT pg_catalog.quote_ident(option_name)"
@@ -3601,7 +3635,8 @@ describeOneTableDetails(const char *schemaname,
}
/* print tables inherited from (exclude partitioned parents) */
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get parent tables"));
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
"WHERE c.oid = i.inhparent AND i.inhrelid = '%s'\n"
@@ -3638,8 +3673,9 @@ describeOneTableDetails(const char *schemaname,
}
/* print child tables (with additional info if partitions) */
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get child tables"));
if (pset.sversion >= 140000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3649,7 +3685,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else if (pset.sversion >= 100000)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending,"
" pg_catalog.pg_get_expr(c.relpartbound, c.oid)\n"
@@ -3659,7 +3695,7 @@ describeOneTableDetails(const char *schemaname,
" c.oid::pg_catalog.regclass::pg_catalog.text;",
oid);
else
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT c.oid::pg_catalog.regclass, c.relkind,"
" false AS inhdetachpending, NULL\n"
"FROM pg_catalog.pg_class c, pg_catalog.pg_inherits i\n"
@@ -3894,7 +3930,8 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching roles"));
+ appendPQExpBuffer(&buf,
"SELECT r.rolname, r.rolsuper, r.rolinherit,\n"
" r.rolcreaterole, r.rolcreatedb, r.rolcanlogin,\n"
" r.rolconnlimit, r.rolvaliduntil");
@@ -4033,7 +4070,8 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf, "SELECT rolname AS \"%s\", datname AS \"%s\",\n"
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get per-database and per-role settings"));
+ appendPQExpBuffer(&buf, "SELECT rolname AS \"%s\", datname AS \"%s\",\n"
"pg_catalog.array_to_string(setconfig, E'\\n') AS \"%s\"\n"
"FROM pg_catalog.pg_db_role_setting s\n"
"LEFT JOIN pg_catalog.pg_database d ON d.oid = setdatabase\n"
@@ -4100,7 +4138,8 @@ describeRoleGrants(const char *pattern, bool showSystem)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching role grants"));
+ appendPQExpBuffer(&buf,
"SELECT m.rolname AS \"%s\", r.rolname AS \"%s\",\n"
" pg_catalog.concat_ws(', ',\n",
gettext_noop("Role name"),
@@ -4199,7 +4238,8 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching relations"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" c.relname as \"%s\",\n"
" CASE c.relkind"
@@ -4482,7 +4522,8 @@ listPartitionedTables(const char *reltypes, const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching partitioned tables"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" c.relname as \"%s\",\n"
" pg_catalog.pg_get_userbyid(c.relowner) as \"%s\"",
@@ -4657,7 +4698,8 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching languages"));
+ appendPQExpBuffer(&buf,
"SELECT l.lanname AS \"%s\",\n"
" pg_catalog.pg_get_userbyid(l.lanowner) as \"%s\",\n"
" l.lanpltrusted AS \"%s\"",
@@ -4733,7 +4775,8 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching domains"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname as \"%s\",\n"
" t.typname as \"%s\",\n"
" pg_catalog.format_type(t.typbasetype, t.typtypmod) as \"%s\",\n"
@@ -4818,7 +4861,8 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching conversions"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname AS \"%s\",\n"
" c.conname AS \"%s\",\n"
" pg_catalog.pg_encoding_to_char(c.conforencoding) AS \"%s\",\n"
@@ -4896,7 +4940,9 @@ describeConfigurationParameters(const char *pattern, bool verbose,
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching configuration parameters"));
+ appendPQExpBuffer(&buf,
"SELECT s.name AS \"%s\", "
"pg_catalog.current_setting(s.name) AS \"%s\"",
gettext_noop("Parameter"),
@@ -4976,7 +5022,8 @@ listEventTriggers(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching event triggers"));
+ appendPQExpBuffer(&buf,
"SELECT evtname as \"%s\", "
"evtevent as \"%s\", "
"pg_catalog.pg_get_userbyid(e.evtowner) as \"%s\",\n"
@@ -5053,7 +5100,9 @@ listExtendedStats(const char *pattern, bool verbose)
}
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching extended statistics"));
+ appendPQExpBuffer(&buf,
"SELECT \n"
"es.stxnamespace::pg_catalog.regnamespace::pg_catalog.text AS \"%s\", \n"
"es.stxname AS \"%s\", \n",
@@ -5146,7 +5195,8 @@ listCasts(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching casts"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.format_type(castsource, NULL) AS \"%s\",\n"
" pg_catalog.format_type(casttarget, NULL) AS \"%s\",\n",
gettext_noop("Source type"),
@@ -5270,7 +5320,8 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching collations"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" n.nspname AS \"%s\",\n"
" c.collname AS \"%s\",\n",
@@ -5393,7 +5444,9 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
char **footers = NULL;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching schemas"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname AS \"%s\",\n"
" pg_catalog.pg_get_userbyid(n.nspowner) AS \"%s\"",
gettext_noop("Name"),
@@ -5436,7 +5489,8 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
PGresult *result;
int i;
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get publications that use this schema"));
+ appendPQExpBuffer(&buf,
"SELECT pubname \n"
"FROM pg_catalog.pg_publication p\n"
" JOIN pg_catalog.pg_publication_namespace pn ON p.oid = pn.pnpubid\n"
@@ -5516,7 +5570,8 @@ listTSParsers(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching text search parsers"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" n.nspname as \"%s\",\n"
" p.prsname as \"%s\",\n"
@@ -5565,7 +5620,8 @@ listTSParsersVerbose(const char *pattern)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching text search parsers"));
+ appendPQExpBuffer(&buf,
"SELECT p.oid,\n"
" n.nspname,\n"
" p.prsname\n"
@@ -5642,7 +5698,8 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get text search parser details"));
+ appendPQExpBuffer(&buf,
"SELECT '%s' AS \"%s\",\n"
" p.prsstart::pg_catalog.regproc AS \"%s\",\n"
" pg_catalog.obj_description(p.prsstart, 'pg_proc') as \"%s\"\n"
@@ -5710,7 +5767,8 @@ describeOneTSParser(const char *oid, const char *nspname, const char *prsname)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get text search parser tokens"));
+ appendPQExpBuffer(&buf,
"SELECT t.alias as \"%s\",\n"
" t.description as \"%s\"\n"
"FROM pg_catalog.ts_token_type( '%s'::pg_catalog.oid ) as t\n"
@@ -5760,7 +5818,8 @@ listTSDictionaries(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching text search dictionaries"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" n.nspname as \"%s\",\n"
" d.dictname as \"%s\",\n",
@@ -5825,8 +5884,9 @@ listTSTemplates(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching text search templates"));
if (verbose)
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT\n"
" n.nspname AS \"%s\",\n"
" t.tmplname AS \"%s\",\n"
@@ -5839,7 +5899,7 @@ listTSTemplates(const char *pattern, bool verbose)
gettext_noop("Lexize"),
gettext_noop("Description"));
else
- printfPQExpBuffer(&buf,
+ appendPQExpBuffer(&buf,
"SELECT\n"
" n.nspname AS \"%s\",\n"
" t.tmplname AS \"%s\",\n"
@@ -5893,7 +5953,8 @@ listTSConfigs(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching text search configurations"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" n.nspname as \"%s\",\n"
" c.cfgname as \"%s\",\n"
@@ -5939,7 +6000,8 @@ listTSConfigsVerbose(const char *pattern)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching text search configurations"));
+ appendPQExpBuffer(&buf,
"SELECT c.oid, c.cfgname,\n"
" n.nspname,\n"
" p.prsname,\n"
@@ -6025,7 +6087,8 @@ describeOneTSConfig(const char *oid, const char *nspname, const char *cfgname,
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get text search configuration details"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" ( SELECT t.alias FROM\n"
" pg_catalog.ts_token_type(c.cfgparser) AS t\n"
@@ -6093,7 +6156,9 @@ listForeignDataWrappers(const char *pattern, bool verbose)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching foreign-data wrappers"));
+ appendPQExpBuffer(&buf,
"SELECT fdw.fdwname AS \"%s\",\n"
" pg_catalog.pg_get_userbyid(fdw.fdwowner) AS \"%s\",\n"
" fdw.fdwhandler::pg_catalog.regproc AS \"%s\",\n"
@@ -6164,7 +6229,9 @@ listForeignServers(const char *pattern, bool verbose)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching foreign servers"));
+ appendPQExpBuffer(&buf,
"SELECT s.srvname AS \"%s\",\n"
" pg_catalog.pg_get_userbyid(s.srvowner) AS \"%s\",\n"
" f.fdwname AS \"%s\"",
@@ -6240,7 +6307,9 @@ listUserMappings(const char *pattern, bool verbose)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching user mappings"));
+ appendPQExpBuffer(&buf,
"SELECT um.srvname AS \"%s\",\n"
" um.usename AS \"%s\"",
gettext_noop("Server"),
@@ -6295,7 +6364,9 @@ listForeignTables(const char *pattern, bool verbose)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching foreign tables"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname AS \"%s\",\n"
" c.relname AS \"%s\",\n"
" s.srvname AS \"%s\"",
@@ -6367,7 +6438,9 @@ listExtensions(const char *pattern)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching extensions"));
+ appendPQExpBuffer(&buf,
"SELECT e.extname AS \"%s\", "
"e.extversion AS \"%s\", ae.default_version AS \"%s\","
"n.nspname AS \"%s\", d.description AS \"%s\"\n"
@@ -6421,7 +6494,9 @@ listExtensionContents(const char *pattern)
int i;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching installed extensions"));
+ appendPQExpBuffer(&buf,
"SELECT e.extname, e.oid\n"
"FROM pg_catalog.pg_extension e\n");
@@ -6489,7 +6564,9 @@ listOneExtensionContents(const char *extname, const char *oid)
printQueryOpt myopt = pset.popt;
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get installed extension details"));
+ appendPQExpBuffer(&buf,
"SELECT pg_catalog.pg_describe_object(classid, objid, 0) AS \"%s\"\n"
"FROM pg_catalog.pg_depend\n"
"WHERE refclassid = 'pg_catalog.pg_extension'::pg_catalog.regclass AND refobjid = '%s' AND deptype = 'e'\n"
@@ -6597,7 +6674,8 @@ listPublications(const char *pattern)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching publications"));
+ appendPQExpBuffer(&buf,
"SELECT pubname AS \"%s\",\n"
" pg_catalog.pg_get_userbyid(pubowner) AS \"%s\",\n"
" puballtables AS \"%s\"",
@@ -6748,7 +6826,8 @@ describePublications(const char *pattern)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get details about matching publications"));
+ appendPQExpBuffer(&buf,
"SELECT oid, pubname,\n"
" pg_catalog.pg_get_userbyid(pubowner) AS owner,\n"
" puballtables");
@@ -6884,8 +6963,8 @@ describePublications(const char *pattern)
if (!puballtables)
{
/* Get the tables for the specified publication */
- printfPQExpBuffer(&buf,
- "SELECT n.nspname, c.relname");
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get tables used by this publication"));
+ appendPQExpBuffer(&buf, "SELECT n.nspname, c.relname");
if (pset.sversion >= 150000)
{
appendPQExpBufferStr(&buf,
@@ -6921,7 +7000,8 @@ describePublications(const char *pattern)
if (pset.sversion >= 150000)
{
/* Get the schemas for the specified publication */
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get schemas used by this publication"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname\n"
"FROM pg_catalog.pg_namespace n\n"
" JOIN pg_catalog.pg_publication_namespace pn ON n.oid = pn.pnnspid\n"
@@ -6937,7 +7017,8 @@ describePublications(const char *pattern)
if (pset.sversion >= 190000)
{
/* Get tables in the EXCEPT clause for this publication */
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get tables excluded by this publication"));
+ appendPQExpBuffer(&buf,
"SELECT n.nspname || '.' || c.relname\n"
"FROM pg_catalog.pg_class c\n"
" JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace\n"
@@ -6997,7 +7078,8 @@ describeSubscriptions(const char *pattern, bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching subscriptions"));
+ appendPQExpBuffer(&buf,
"SELECT subname AS \"%s\"\n"
", pg_catalog.pg_get_userbyid(subowner) AS \"%s\"\n"
", subenabled AS \"%s\"\n"
@@ -7166,7 +7248,8 @@ listOperatorClasses(const char *access_method_pattern,
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching operator classes"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" am.amname AS \"%s\",\n"
" pg_catalog.format_type(c.opcintype, NULL) AS \"%s\",\n"
@@ -7267,7 +7350,8 @@ listOperatorFamilies(const char *access_method_pattern,
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching operator families"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" am.amname AS \"%s\",\n"
" CASE\n"
@@ -7357,7 +7441,8 @@ listOpFamilyOperators(const char *access_method_pattern,
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get operators of matching operator families"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" am.amname AS \"%s\",\n"
" CASE\n"
@@ -7463,7 +7548,8 @@ listOpFamilyFunctions(const char *access_method_pattern,
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get matching support functions of operator families"));
+ appendPQExpBuffer(&buf,
"SELECT\n"
" am.amname AS \"%s\",\n"
" CASE\n"
@@ -7549,7 +7635,8 @@ listLargeObjects(bool verbose)
initPQExpBuffer(&buf);
- printfPQExpBuffer(&buf,
+ printfPQExpBuffer(&buf, "/* %s */\n", _("Get large objects"));
+ appendPQExpBuffer(&buf,
"SELECT oid as \"%s\",\n"
" pg_catalog.pg_get_userbyid(lomowner) as \"%s\",\n ",
gettext_noop("ID"),
--
2.47.3
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2026-03-23 22:50 Greg Sabino Mullane <[email protected]>
parent: Greg Sabino Mullane <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Greg Sabino Mullane @ 2026-03-23 22:50 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Maiquel Grassi <[email protected]>; David Christensen <[email protected]>; Peter Eisentraut <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
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
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: Adding comments to help understand psql hidden queries
@ 2026-03-26 15:41 Tom Lane <[email protected]>
parent: Greg Sabino Mullane <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Tom Lane @ 2026-03-26 15:41 UTC (permalink / raw)
To: Greg Sabino Mullane <[email protected]>; +Cc: Maiquel Grassi <[email protected]>; David Christensen <[email protected]>; Peter Eisentraut <[email protected]>; Jim Jones <[email protected]>; pgsql-hackers
I wrote:
> Greg Sabino Mullane <[email protected]> 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
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2026-03-26 15:41 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-22 15:39 Re: Adding comments to help understand psql hidden queries David Christensen <[email protected]>
2024-03-22 17:37 ` Greg Sabino Mullane <[email protected]>
2024-04-03 17:16 ` David Christensen <[email protected]>
2024-04-04 14:31 ` Peter Eisentraut <[email protected]>
2024-04-04 16:12 ` David Christensen <[email protected]>
2024-06-11 22:54 ` David Christensen <[email protected]>
2025-01-18 20:37 ` Tom Lane <[email protected]>
2025-01-30 20:55 ` David Christensen <[email protected]>
2025-04-01 00:02 ` Maiquel Grassi <[email protected]>
2026-03-22 15:15 ` Greg Sabino Mullane <[email protected]>
2026-03-23 22:50 ` Greg Sabino Mullane <[email protected]>
2026-03-26 15:41 ` Tom Lane <[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