On Wed, 28 Dec 2022 at 21:26, Nathan Bossart <nathandboss...@gmail.com> wrote:
>
> On Wed, Dec 28, 2022 at 02:46:23PM +0300, Maxim Orlov wrote:
> > The patch applies with no problem, implements what it declared, CF bot is
> > happy.
> > Without patch \dpS shows 0 rows, after applying system objects are shown.
> > Consider this patch useful, hope it will be committed soon.
>
> Thanks for reviewing.
>
Looking this over this, I have a couple of comments:
Firstly, I think it should allow \zS in the same fashion as \dpS,
since \z is an alias for \dp, so the 2 should be kept in sync.
Secondly, I don't think the following is the right SQL clause to use
in the absence of "S":
if (!showSystem && !pattern)
appendPQExpBufferStr(&buf, "AND n.nspname !~ '^pg_'\n");
I know that's the condition it used before, but the problem with using
that now is that it will cause temporary relations to be excluded
unless the "S" modifier is used, which goes against the expectation
that "S" just causes system relations to be included. Also, it fails
to exclude information_schema relations, if that happens to be on the
user's search_path.
So I think we should use the same SQL clauses as every other psql
command that supports "S", namely:
if (!showSystem && !pattern)
appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
" AND n.nspname <> 'information_schema'\n");
Updated patch attached.
Regards,
Dean
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
new file mode 100644
index 8a5285d..3f994a3
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -1825,14 +1825,16 @@ INSERT INTO tbl1 VALUES ($1, $2) \bind '
<varlistentry>
- <term><literal>\dp [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <term><literal>\dp[S] [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
<listitem>
<para>
Lists tables, views and sequences with their
associated access privileges.
If <replaceable class="parameter">pattern</replaceable> is
specified, only tables, views and sequences whose names match the
- pattern are listed.
+ pattern are listed. By default only user-created objects are shown;
+ supply a pattern or the <literal>S</literal> modifier to include
+ system objects.
</para>
<para>
@@ -3575,14 +3577,16 @@ testdb=> <userinput>\setenv LESS -imx
<varlistentry>
- <term><literal>\z [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
+ <term><literal>\z[S] [ <link linkend="app-psql-patterns"><replaceable class="parameter">pattern</replaceable></link> ]</literal></term>
<listitem>
<para>
Lists tables, views and sequences with their
associated access privileges.
If a <replaceable class="parameter">pattern</replaceable> is
specified, only tables, views and sequences whose names match the
- pattern are listed.
+ pattern are listed. By default only user-created objects are shown;
+ supply a pattern or the <literal>S</literal> modifier to include
+ system objects.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 00b89d9..b5201ed
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -140,7 +140,8 @@ static backslashResult exec_command_writ
static backslashResult exec_command_watch(PsqlScanState scan_state, bool active_branch,
PQExpBuffer query_buf, PQExpBuffer previous_buf);
static backslashResult exec_command_x(PsqlScanState scan_state, bool active_branch);
-static backslashResult exec_command_z(PsqlScanState scan_state, bool active_branch);
+static backslashResult exec_command_z(PsqlScanState scan_state, bool active_branch,
+ const char *cmd);
static backslashResult exec_command_shell_escape(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_slash_command_help(PsqlScanState scan_state, bool active_branch);
static char *read_connect_arg(PsqlScanState scan_state);
@@ -413,8 +414,8 @@ exec_command(const char *cmd,
query_buf, previous_buf);
else if (strcmp(cmd, "x") == 0)
status = exec_command_x(scan_state, active_branch);
- else if (strcmp(cmd, "z") == 0)
- status = exec_command_z(scan_state, active_branch);
+ else if (strcmp(cmd, "z") == 0 || strcmp(cmd, "zS") == 0)
+ status = exec_command_z(scan_state, active_branch, cmd);
else if (strcmp(cmd, "!") == 0)
status = exec_command_shell_escape(scan_state, active_branch);
else if (strcmp(cmd, "?") == 0)
@@ -875,7 +876,7 @@ exec_command_d(PsqlScanState scan_state,
success = listCollations(pattern, show_verbose, show_system);
break;
case 'p':
- success = permissionsList(pattern);
+ success = permissionsList(pattern, show_system);
break;
case 'P':
{
@@ -2822,16 +2823,22 @@ exec_command_x(PsqlScanState scan_state,
* \z -- list table privileges (equivalent to \dp)
*/
static backslashResult
-exec_command_z(PsqlScanState scan_state, bool active_branch)
+exec_command_z(PsqlScanState scan_state, bool active_branch, const char *cmd)
{
bool success = true;
if (active_branch)
{
- char *pattern = psql_scan_slash_option(scan_state,
- OT_NORMAL, NULL, true);
+ char *pattern;
+ bool show_system;
+
+ pattern = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, true);
+
+ show_system = strchr(cmd, 'S') ? true : false;
+
+ success = permissionsList(pattern, show_system);
- success = permissionsList(pattern);
free(pattern);
}
else
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
new file mode 100644
index 523fab6..e280b6f
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1002,7 +1002,7 @@ listAllDbs(const char *pattern, bool ver
* \z (now also \dp -- perhaps more mnemonic)
*/
bool
-permissionsList(const char *pattern)
+permissionsList(const char *pattern, bool showSystem)
{
PQExpBufferData buf;
PGresult *res;
@@ -1121,15 +1121,13 @@ permissionsList(const char *pattern)
CppAsString2(RELKIND_FOREIGN_TABLE) ","
CppAsString2(RELKIND_PARTITIONED_TABLE) ")\n");
- /*
- * Unless a schema pattern is specified, we suppress system and temp
- * tables, since they normally aren't very interesting from a permissions
- * point of view. You can see 'em by explicit request though, eg with \z
- * pg_catalog.*
- */
+ if (!showSystem && !pattern)
+ appendPQExpBufferStr(&buf, " AND n.nspname <> 'pg_catalog'\n"
+ " AND n.nspname <> 'information_schema'\n");
+
if (!validateSQLNamePattern(&buf, pattern, true, false,
"n.nspname", "c.relname", NULL,
- "n.nspname !~ '^pg_' AND pg_catalog.pg_table_is_visible(c.oid)",
+ "pg_catalog.pg_table_is_visible(c.oid)",
NULL, 3))
goto error_return;
diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h
new file mode 100644
index 15f62b9..554fe86
--- a/src/bin/psql/describe.h
+++ b/src/bin/psql/describe.h
@@ -38,7 +38,7 @@ extern bool describeRoles(const char *pa
extern bool listDbRoleSettings(const char *pattern, const char *pattern2);
/* \z (or \dp) */
-extern bool permissionsList(const char *pattern);
+extern bool permissionsList(const char *pattern, bool showSystem);
/* \ddp */
extern bool listDefaultACLs(const char *pattern);
