Adding error messages to a few slash commands
Hello hackers,
Currently, some slash commands in psql return an error saying "Did not
find any XXXX named YYYY" while some return an empty table. This patch
changes a few of the slash commands to return a similar error message.
I did not change all of the slash commands in this initial patch to
wait for feedback, happy to do that if this patch is acceptable. I did
not add any tests yet because the existing ones did not seem to have
any, happy to add tests if needed. Also, I know that we are in a
feature freeze, is such a change acceptable now?
Thanks
--
Thanks and regards
Abhishek Chanda
Attachments:
v1-0001-Add-an-error-message-when-a-given-object-is-not-f.patchapplication/octet-stream; name=v1-0001-Add-an-error-message-when-a-given-object-is-not-f.patchDownload
From d0c38dd4debb5b3f4c6a0583a7c548fe7c88fad4 Mon Sep 17 00:00:00 2001
From: Abhishek Chanda <abhishek.becs@gmail.com>
Date: Sat, 12 Apr 2025 22:30:17 -0500
Subject: [PATCH v1] Add an error message when a given object is not found in a
slash command
Currently, in a few cases we return an error of the form
Did not find any XXXX named YYYY while in some cases we
print a table with no rows. This patch changes a few of
the later to the former so that we have an uniform
interface.
---
src/bin/psql/describe.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 1d08268393e..4e929504112 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -605,6 +605,14 @@ describeFunctions(const char *functypes, const char *func_pattern,
if (!res)
return false;
+ /* If there are no matching functions, print a message */
+ if (PQntuples(res) == 0 && func_pattern)
+ {
+ PQclear(res);
+ pg_log_error("Did not find any function named \"%s\".", func_pattern);
+ return true;
+ }
+
myopt.title = _("List of functions");
myopt.translate_header = true;
if (pset.sversion >= 90600)
@@ -721,6 +729,14 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
if (!res)
return false;
+ /* If there are no matching data types, print a message */
+ if (PQntuples(res) == 0 && pattern)
+ {
+ PQclear(res);
+ pg_log_error("Did not find any data type named \"%s\".", pattern);
+ return true;
+ }
+
myopt.title = _("List of data types");
myopt.translate_header = true;
@@ -3762,6 +3778,15 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
return false;
nrows = PQntuples(res);
+
+ /* If there are no matching roles, print a message */
+ if (nrows == 0 && pattern)
+ {
+ PQclear(res);
+ pg_log_error("Did not find any role named \"%s\".", pattern);
+ return true;
+ }
+
attr = pg_malloc0((nrows + 1) * sizeof(*attr));
printTableInit(&cont, &myopt, _("List of roles"), ncols, nrows);
@@ -5242,6 +5267,14 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
if (!res)
goto error_return;
+ /* If there are no matching schemas, print a message */
+ if (PQntuples(res) == 0 && pattern)
+ {
+ PQclear(res);
+ pg_log_error("Did not find any schema named \"%s\".", pattern);
+ return true;
+ }
+
myopt.title = _("List of schemas");
myopt.translate_header = true;
@@ -6213,6 +6246,14 @@ listExtensions(const char *pattern)
if (!res)
return false;
+ /* If there are no matching extensions, print a message */
+ if (PQntuples(res) == 0 && pattern)
+ {
+ PQclear(res);
+ pg_log_error("Did not find any extension named \"%s\".", pattern);
+ return true;
+ }
+
myopt.title = _("List of installed extensions");
myopt.translate_header = true;
--
2.49.0
Sorry, I forgot to attach example usage. Here is how these currently behave:
postgres=> \dT foo
List of data types
Schema | Name | Description
--------+------+-------------
(0 rows)
postgres => \du foo
List of roles
Role name | Attributes
-----------+------------
postgres => \df foo
List of functions
Schema | Name | Result data type | Argument data types | Type
--------+------+------------------+---------------------+------
(0 rows)
Here is how these look like after this change:
postgres=> \dT foo
Did not find any data type named "foo".
postgres => \df foo
Did not find any function named "foo".
postgres => \du foo
Did not find any role named "foo".
Thanks
On Sat, Apr 12, 2025 at 10:43 PM Abhishek Chanda
<abhishek.becs@gmail.com> wrote:
Hello hackers,
Currently, some slash commands in psql return an error saying "Did not
find any XXXX named YYYY" while some return an empty table. This patch
changes a few of the slash commands to return a similar error message.
I did not change all of the slash commands in this initial patch to
wait for feedback, happy to do that if this patch is acceptable. I did
not add any tests yet because the existing ones did not seem to have
any, happy to add tests if needed. Also, I know that we are in a
feature freeze, is such a change acceptable now?Thanks
--
Thanks and regards
Abhishek Chanda
--
Thanks and regards
Abhishek Chanda
Abhishek Chanda <abhishek.becs@gmail.com> writes:
Currently, some slash commands in psql return an error saying "Did not
find any XXXX named YYYY" while some return an empty table. This patch
changes a few of the slash commands to return a similar error message.
Personally, if I were trying to make these things consistent, I'd have
gone in the other direction (ie return empty tables). We don't make
psql throw an error when an ordinary user query returns zero rows;
why should \d commands do that?
Also, I know that we are in a
feature freeze, is such a change acceptable now?
Whether changing this is a good idea or not, it's surely hard
to claim that it's a bug fix. So I'd say it's out of scope for
post-feature-freeze.
regards, tom lane
On 13.04.2025 08:29, Tom Lane wrote:
Abhishek Chanda<abhishek.becs@gmail.com> writes:
Currently, some slash commands in psql return an error saying "Did not
find any XXXX named YYYY" while some return an empty table. This patch
changes a few of the slash commands to return a similar error message.
+1 for this patch.
Personally, if I were trying to make these things consistent, I'd have
gone in the other direction (ie return empty tables).
+1
Returning empty tables is a more appropriate behavior.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Thanks for the feedback, attached an updated patch that changes most
of those "Did not find" messages to empty tables. I did not change two
sets, listDbRoleSettings and listTables both have comments describing
that these are deliberately this way.
I wanted to post this update to close this loop for now. I will follow
up once the merge window opens again.
Thanks
On Sun, Apr 13, 2025 at 1:47 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
On 13.04.2025 08:29, Tom Lane wrote:
Abhishek Chanda <abhishek.becs@gmail.com> writes:
Currently, some slash commands in psql return an error saying "Did not
find any XXXX named YYYY" while some return an empty table. This patch
changes a few of the slash commands to return a similar error message.+1 for this patch.
Personally, if I were trying to make these things consistent, I'd have
gone in the other direction (ie return empty tables).+1
Returning empty tables is a more appropriate behavior.--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
--
Thanks and regards
Abhishek Chanda
Attachments:
v2-0001-Print-empty-table-when-a-given-object-is-not-foun.patchapplication/octet-stream; name=v2-0001-Print-empty-table-when-a-given-object-is-not-foun.patchDownload
From 36377dd05b7743a879606bc36c10a9d6c12a7fc5 Mon Sep 17 00:00:00 2001
From: Abhishek Chanda <abhishek.becs@gmail.com>
Date: Sat, 12 Apr 2025 22:30:17 -0500
Subject: [PATCH v2] Print empty table when a given object is not found in a
slash command
Currently, in a few cases we return an error of the form
Did not find any XXXX named YYYY while in some cases we
print a table with no rows. This patch changes a few of
the former to the later so that we have an uniform
interface.
---
src/bin/psql/describe.c | 81 +----------------------------------------
1 file changed, 1 insertion(+), 80 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 1d08268393e..ebb5d3b0c69 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -1519,20 +1519,6 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
if (!res)
return false;
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any relation named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any relations.");
- }
- PQclear(res);
- return false;
- }
-
for (i = 0; i < PQntuples(res); i++)
{
const char *oid;
@@ -1719,14 +1705,6 @@ describeOneTableDetails(const char *schemaname,
if (!res)
goto error_return;
- /* Did we get anything? */
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- pg_log_error("Did not find any relation with OID %s.", oid);
- goto error_return;
- }
-
tableinfo.checks = atoi(PQgetvalue(res, 0, 0));
tableinfo.relkind = *(PQgetvalue(res, 0, 1));
tableinfo.hasindex = strcmp(PQgetvalue(res, 0, 2), "t") == 0;
@@ -3762,6 +3740,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
return false;
nrows = PQntuples(res);
+
attr = pg_malloc0((nrows + 1) * sizeof(*attr));
printTableInit(&cont, &myopt, _("List of roles"), ncols, nrows);
@@ -5403,20 +5382,6 @@ listTSParsersVerbose(const char *pattern)
if (!res)
return false;
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any text search parser named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any text search parsers.");
- }
- PQclear(res);
- return false;
- }
-
for (i = 0; i < PQntuples(res); i++)
{
const char *oid;
@@ -5781,20 +5746,6 @@ listTSConfigsVerbose(const char *pattern)
if (!res)
return false;
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any text search configuration named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any text search configurations.");
- }
- PQclear(res);
- return false;
- }
-
for (i = 0; i < PQntuples(res); i++)
{
const char *oid;
@@ -6256,20 +6207,6 @@ listExtensionContents(const char *pattern)
if (!res)
return false;
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any extension named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any extensions.");
- }
- PQclear(res);
- return false;
- }
-
for (i = 0; i < PQntuples(res); i++)
{
const char *extname;
@@ -6603,22 +6540,6 @@ describePublications(const char *pattern)
return false;
}
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any publication named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any publications.");
- }
-
- termPQExpBuffer(&buf);
- PQclear(res);
- return false;
- }
-
for (i = 0; i < PQntuples(res); i++)
{
const char align = 'l';
--
2.49.0
On 13.04.2025 19:40, Abhishek Chanda wrote:
Thanks for the feedback, attached an updated patch that changes most
of those "Did not find" messages to empty tables. I did not change two
sets, listDbRoleSettings and listTables both have comments describing
that these are deliberately this way.
Thanks.
I wanted to post this update to close this loop for now. I will follow
up once the merge window opens again.
I recommend to create a new entry for this patch in the open July
commitfest <https://commitfest.postgresql.org/53/>.
I will be able to review this patch in a couple months later in June,
if no one wants to review it earlier.
--
Pavel Luzanov
Postgres Professional:https://postgrespro.com
Thanks Pavel, done now https://commitfest.postgresql.org/patch/5699/
Thanks
On Mon, Apr 14, 2025 at 4:27 AM Pavel Luzanov <p.luzanov@postgrespro.ru> wrote:
On 13.04.2025 19:40, Abhishek Chanda wrote:
Thanks for the feedback, attached an updated patch that changes most
of those "Did not find" messages to empty tables. I did not change two
sets, listDbRoleSettings and listTables both have comments describing
that these are deliberately this way.Thanks.
I wanted to post this update to close this loop for now. I will follow
up once the merge window opens again.I recommend to create a new entry for this patch in the open July commitfest.
I will be able to review this patch in a couple months later in June,
if no one wants to review it earlier.--
Pavel Luzanov
Postgres Professional: https://postgrespro.com
--
Thanks and regards
Abhishek Chanda
On Mon Apr 14, 2025 at 12:27:53 GMT +03, Pavel Luzanov wrote:
I recommend to create a new entry for this patch in the open July
commitfest <https://commitfest.postgresql.org/53/>.
I will be able to review this patch in a couple months later in June,
if no one wants to review it earlier.
I could review it right now, i.e. do a pre-commit review according to [1]https://wiki.postgresql.org/wiki/Reviewing_a_Patch.
Still have to "pay off" my own Commitfest entry. This would be my first PG
review. But is it even desirable to write reviews before the beginning of the
Commitfest? An important part -- especially in simple patches like this one
-- would be to apply it to HEAD. And shouldn't that be better done as late as
possible?
[1]: https://wiki.postgresql.org/wiki/Reviewing_a_Patch
--
Robin Haberkorn
Senior Software Engineer
B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / https://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt, HRB 3537
On Sun, 13 Apr 2025 at 17:40, Abhishek Chanda <abhishek.becs@gmail.com> wrote:
Thanks for the feedback, attached an updated patch that changes most
of those "Did not find" messages to empty tables. I did not change two
sets, listDbRoleSettings and listTables both have comments describing
that these are deliberately this way.
+1 for always displaying an empty table if no objects are found.
The reason given for listDbRoleSettings() being an exception could be
solved by making the title depend on the number of arguments, e.g.,
"List of settings for role \"%s\" and database \"%s\".".
It's not clear what the "historical reasons" are for listTables()
being an exception, but if we're changing any of these, I'd say we
should make them all consistent.
Regards,
Dean
Hello Abhishek,
I have reviewed your patch after there was no response to my initial proposal
and nobody else wrote a review yet.
The patch is in the correct unified-diff format (apparently generated via git
format-patch). It applies cleanly to the current master branch. The patch does
not however add any tests (e.g. to src/bin/psql/t/001_basic.pl). On the other
hand, there weren't any tests for the affected slash-commands, so the patch
doesn't break the test suite.
The patch tries to remove special "Did not find any XXXX named YYYY" error
messages for `\d` commands (`\d`, `\dx+`, `\dRp+`, `\dFp` and `\dF`) in psql,
printing empty tables instead. This would make the behavior of the `\d`
commands more consistent with the the behavior of ordinary user queries. On the
other hand, the change in question could well break existing scripts calling
`psql -c '\d ...'`, especially since the process return code changes. For
instance, before the proposed change, psql would fail with return code 1:
# psql -c '\d foo'; echo $?
Did not find any relation named "foo".
1
After applying the patch, the return code will be 0 (success) instead:
# psql -c '\d foo'; echo $?
0
I would suggest to rework the patch, so that 1 is still returned. This is also
in line with how UNIX tools like `grep` behave in case they report an "empty"
response (i.e. if `grep` does not produce any match in the given files). On the
other hand returning 1 without printing any error message might create new
inconsistencies in psql. More feedback is required from the community on that
question. If the return code is considered important by the community, it would
be a good reason for adding a test case, so that psql behavior doesn't change
unexpectedly in the future.
It is noteworthy, that both before and after the patch, a plain `\d` does
output an error message while the psql process still returns with code 0:
# psql -c '\d'; echo $?
Did not find any relations.
0
You clarified you didn't remove the messages because of code comments in
listTables() and listDbRoleSettings(). A failure return code however should
probably still be returned (if we decide that this is what all the other \d
commands should do).
The examples above also raise another possible issue. The commands `\d`, `\dx+`
and `\dRp+` actually do not print empty tables (instead of "Did not find any
XXXX named YYYY") when invoked with a pattern, but produce no output at all.
This probably makes sense considering that they could also print output for
multiple items (e.g. tables). The remaining affected commands (`\dFp` and
`\dF`) will actually print empty tables now.
Another point of criticism is that the patch needlessly adds an empty line in
describeRoles() - moreover a line with trailing whitespace. I would suggest to
remove that change before committing.
To summarize, in my opinion you should:
* Get feedback on the desired return codes of psql in case of
empty responses.
* Fix the return codes if others agree that psql should return failure codes
and add a test case.
* Remove the unnecessary change in describeRoles().
Best regards,
Robin Haberkorn
On Tue May 13, 2025 at 00:00:17 GMT +03, Robin Haberkorn wrote:
On Mon Apr 14, 2025 at 12:27:53 GMT +03, Pavel Luzanov wrote:
I recommend to create a new entry for this patch in the open July
commitfest <https://commitfest.postgresql.org/53/>.
I will be able to review this patch in a couple months later in June,
if no one wants to review it earlier.I could review it right now, i.e. do a pre-commit review according to [1].
Still have to "pay off" my own Commitfest entry. This would be my first PG
review. But is it even desirable to write reviews before the beginning of the
Commitfest? An important part -- especially in simple patches like this one
-- would be to apply it to HEAD. And shouldn't that be better done as late as
possible?
--
Robin Haberkorn
Software Engineer
B1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / https://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt, HRB 3537
Hi all,
Thanks a lot for the review, Robin. And I am terribly sorry for the
slow reply, it just took me a while to get to this. But I think I have
addressed all your feedback, I have changed everything to set an error
code of 1 if anything is not empty. Also added tests for the return
code as requested, and cleaned up the change in describeRoles().
Please let me know if I missed anything.
Attached an updated and rebased version of the patch.
Thanks
On Fri, Jul 11, 2025 at 4:15 AM Robin Haberkorn <haberkorn@b1-systems.de> wrote:
Hello Abhishek,
I have reviewed your patch after there was no response to my initial proposal
and nobody else wrote a review yet.The patch is in the correct unified-diff format (apparently generated via git
format-patch). It applies cleanly to the current master branch. The patch does
not however add any tests (e.g. to src/bin/psql/t/001_basic.pl). On the other
hand, there weren't any tests for the affected slash-commands, so the patch
doesn't break the test suite.The patch tries to remove special "Did not find any XXXX named YYYY" error
messages for `\d` commands (`\d`, `\dx+`, `\dRp+`, `\dFp` and `\dF`) in psql,
printing empty tables instead. This would make the behavior of the `\d`
commands more consistent with the the behavior of ordinary user queries. On the
other hand, the change in question could well break existing scripts calling
`psql -c '\d ...'`, especially since the process return code changes. For
instance, before the proposed change, psql would fail with return code 1:# psql -c '\d foo'; echo $?
Did not find any relation named "foo".
1After applying the patch, the return code will be 0 (success) instead:
# psql -c '\d foo'; echo $?
0I would suggest to rework the patch, so that 1 is still returned. This is also
in line with how UNIX tools like `grep` behave in case they report an "empty"
response (i.e. if `grep` does not produce any match in the given files). On the
other hand returning 1 without printing any error message might create new
inconsistencies in psql. More feedback is required from the community on that
question. If the return code is considered important by the community, it would
be a good reason for adding a test case, so that psql behavior doesn't change
unexpectedly in the future.It is noteworthy, that both before and after the patch, a plain `\d` does
output an error message while the psql process still returns with code 0:# psql -c '\d'; echo $?
Did not find any relations.
0You clarified you didn't remove the messages because of code comments in
listTables() and listDbRoleSettings(). A failure return code however should
probably still be returned (if we decide that this is what all the other \d
commands should do).The examples above also raise another possible issue. The commands `\d`, `\dx+`
and `\dRp+` actually do not print empty tables (instead of "Did not find any
XXXX named YYYY") when invoked with a pattern, but produce no output at all.
This probably makes sense considering that they could also print output for
multiple items (e.g. tables). The remaining affected commands (`\dFp` and
`\dF`) will actually print empty tables now.Another point of criticism is that the patch needlessly adds an empty line in
describeRoles() - moreover a line with trailing whitespace. I would suggest to
remove that change before committing.To summarize, in my opinion you should:
* Get feedback on the desired return codes of psql in case of
empty responses.
* Fix the return codes if others agree that psql should return failure codes
and add a test case.
* Remove the unnecessary change in describeRoles().Best regards,
Robin HaberkornOn Tue May 13, 2025 at 00:00:17 GMT +03, Robin Haberkorn wrote:
On Mon Apr 14, 2025 at 12:27:53 GMT +03, Pavel Luzanov wrote:
I recommend to create a new entry for this patch in the open July
commitfest <https://commitfest.postgresql.org/53/>.
I will be able to review this patch in a couple months later in June,
if no one wants to review it earlier.I could review it right now, i.e. do a pre-commit review according to [1].
Still have to "pay off" my own Commitfest entry. This would be my first PG
review. But is it even desirable to write reviews before the beginning of the
Commitfest? An important part -- especially in simple patches like this one
-- would be to apply it to HEAD. And shouldn't that be better done as late as
possible?--
Robin Haberkorn
Software EngineerB1 Systems GmbH
Osterfeldstraße 7 / 85088 Vohburg / https://www.b1-systems.de
GF: Ralph Dehner / Unternehmenssitz: Vohburg / AG: Ingolstadt, HRB 3537
--
Thanks and regards
Abhishek Chanda
Attachments:
v3-0001-Print-empty-table-when-a-given-object-is-not-foun.patchapplication/octet-stream; name=v3-0001-Print-empty-table-when-a-given-object-is-not-foun.patchDownload
From 86ea629bc4ede9c51f9c000caddf1969b593b569 Mon Sep 17 00:00:00 2001
From: Abhishek Chanda <abhishek.becs@gmail.com>
Date: Sat, 12 Apr 2025 22:30:17 -0500
Subject: [PATCH v3] Print empty table when a given object is not found in a
slash command
Currently, in a few cases we return an error of the form
Did not find any XXXX named YYYY while in some cases we
print a table with no rows. This patch changes a few of
the former to the later so that we have an uniform
interface.
---
src/bin/psql/describe.c | 377 ++++++++++++++++++++++--------------
src/bin/psql/t/001_basic.pl | 26 +++
2 files changed, 258 insertions(+), 145 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 36f24502842..de1bb31b231 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -136,6 +136,12 @@ describeAggregates(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -210,6 +216,12 @@ describeAccessMethods(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -272,6 +284,12 @@ describeTablespaces(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -621,6 +639,12 @@ describeFunctions(const char *functypes, const char *func_pattern,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (func_pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
@@ -727,6 +751,12 @@ describeTypes(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -928,6 +958,12 @@ describeOperators(const char *oper_pattern,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (oper_pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
@@ -1037,6 +1073,12 @@ listAllDbs(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -1270,6 +1312,13 @@ listDefaultACLs(const char *pattern)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ termPQExpBuffer(&buf);
+ PQclear(res);
+ return false;
+ }
+
termPQExpBuffer(&buf);
PQclear(res);
return true;
@@ -1522,16 +1571,14 @@ describeTableDetails(const char *pattern, bool verbose, bool showSystem)
if (PQntuples(res) == 0)
{
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any relation named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any relations.");
- }
PQclear(res);
- return false;
+ /*
+ * Print an empty table and return failure when no objects match
+ */
+ if (pattern)
+ return listTables("tvmsE", pattern, verbose, showSystem);
+ else
+ return true;
}
for (i = 0; i < PQntuples(res); i++)
@@ -1720,14 +1767,6 @@ describeOneTableDetails(const char *schemaname,
if (!res)
goto error_return;
- /* Did we get anything? */
- if (PQntuples(res) == 0)
- {
- if (!pset.quiet)
- pg_log_error("Did not find any relation with OID %s.", oid);
- goto error_return;
- }
-
tableinfo.checks = atoi(PQgetvalue(res, 0, 0));
tableinfo.relkind = *(PQgetvalue(res, 0, 1));
tableinfo.hasindex = strcmp(PQgetvalue(res, 0, 2), "t") == 0;
@@ -3797,6 +3836,7 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
return false;
nrows = PQntuples(res);
+
attr = pg_malloc0((nrows + 1) * sizeof(*attr));
printTableInit(&cont, &myopt, _("List of roles"), ncols, nrows);
@@ -3873,6 +3913,12 @@ describeRoles(const char *pattern, bool verbose, bool showSystem)
free(attr[i]);
free(attr);
+ if (pattern && nrows == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -3921,29 +3967,15 @@ listDbRoleSettings(const char *pattern, const char *pattern2)
if (!res)
return false;
- /*
- * Most functions in this file are content to print an empty table when
- * there are no matching objects. We intentionally deviate from that
- * here, but only in !quiet mode, because of the possibility that the user
- * is confused about what the two pattern arguments mean.
- */
- if (PQntuples(res) == 0 && !pset.quiet)
- {
- if (pattern && pattern2)
- pg_log_error("Did not find any settings for role \"%s\" and database \"%s\".",
- pattern, pattern2);
- else if (pattern)
- pg_log_error("Did not find any settings for role \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any settings.");
- }
- else
- {
- myopt.title = _("List of settings");
- myopt.translate_header = true;
+ myopt.title = _("List of settings");
+ myopt.translate_header = true;
- printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+
+ if ((pattern || pattern2) && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
}
PQclear(res);
@@ -4018,6 +4050,12 @@ describeRoleGrants(const char *pattern, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -4201,76 +4239,25 @@ listTables(const char *tabtypes, const char *pattern, bool verbose, bool showSys
if (!res)
return false;
- /*
- * Most functions in this file are content to print an empty table when
- * there are no matching objects. We intentionally deviate from that
- * here, but only in !quiet mode, for historical reasons.
- */
- if (PQntuples(res) == 0 && !pset.quiet)
- {
- if (pattern)
- {
- if (ntypes != 1)
- pg_log_error("Did not find any relations named \"%s\".",
- pattern);
- else if (showTables)
- pg_log_error("Did not find any tables named \"%s\".",
- pattern);
- else if (showIndexes)
- pg_log_error("Did not find any indexes named \"%s\".",
- pattern);
- else if (showViews)
- pg_log_error("Did not find any views named \"%s\".",
- pattern);
- else if (showMatViews)
- pg_log_error("Did not find any materialized views named \"%s\".",
- pattern);
- else if (showSeq)
- pg_log_error("Did not find any sequences named \"%s\".",
- pattern);
- else if (showForeign)
- pg_log_error("Did not find any foreign tables named \"%s\".",
- pattern);
- else /* should not get here */
- pg_log_error_internal("Did not find any ??? named \"%s\".",
- pattern);
- }
- else
- {
- if (ntypes != 1)
- pg_log_error("Did not find any relations.");
- else if (showTables)
- pg_log_error("Did not find any tables.");
- else if (showIndexes)
- pg_log_error("Did not find any indexes.");
- else if (showViews)
- pg_log_error("Did not find any views.");
- else if (showMatViews)
- pg_log_error("Did not find any materialized views.");
- else if (showSeq)
- pg_log_error("Did not find any sequences.");
- else if (showForeign)
- pg_log_error("Did not find any foreign tables.");
- else /* should not get here */
- pg_log_error_internal("Did not find any ??? relations.");
- }
- }
- else
- {
- myopt.title =
- (ntypes != 1) ? _("List of relations") :
- (showTables) ? _("List of tables") :
- (showIndexes) ? _("List of indexes") :
- (showViews) ? _("List of views") :
- (showMatViews) ? _("List of materialized views") :
- (showSeq) ? _("List of sequences") :
- (showForeign) ? _("List of foreign tables") :
- "List of ???"; /* should not get here */
- myopt.translate_header = true;
- myopt.translate_columns = translate_columns;
- myopt.n_translate_columns = lengthof(translate_columns);
+ myopt.title =
+ (ntypes != 1) ? _("List of relations") :
+ (showTables) ? _("List of tables") :
+ (showIndexes) ? _("List of indexes") :
+ (showViews) ? _("List of views") :
+ (showMatViews) ? _("List of materialized views") :
+ (showSeq) ? _("List of sequences") :
+ (showForeign) ? _("List of foreign tables") :
+ "List of ???"; /* should not get here */
+ myopt.translate_header = true;
+ myopt.translate_columns = translate_columns;
+ myopt.n_translate_columns = lengthof(translate_columns);
- printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
}
PQclear(res);
@@ -4568,6 +4555,12 @@ listLanguages(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -4652,6 +4645,12 @@ listDomains(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -4732,6 +4731,12 @@ listConversions(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -4800,6 +4805,12 @@ describeConfigurationParameters(const char *pattern, bool verbose,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -4880,6 +4891,12 @@ listEventTriggers(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -4976,6 +4993,12 @@ listExtendedStats(const char *pattern)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -5223,6 +5246,12 @@ listCollations(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -5327,6 +5356,13 @@ listSchemas(const char *pattern, bool verbose, bool showSystem)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ termPQExpBuffer(&buf);
+ PQclear(res);
+ return false;
+ }
+
termPQExpBuffer(&buf);
PQclear(res);
@@ -5398,6 +5434,12 @@ listTSParsers(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -5440,16 +5482,11 @@ listTSParsersVerbose(const char *pattern)
if (PQntuples(res) == 0)
{
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any text search parser named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any text search parsers.");
- }
PQclear(res);
- return false;
+ if (pattern)
+ return listTSParsers(pattern, false);
+ else
+ return true;
}
for (i = 0; i < PQntuples(res); i++)
@@ -5775,6 +5812,12 @@ listTSConfigs(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -5818,16 +5861,11 @@ listTSConfigsVerbose(const char *pattern)
if (PQntuples(res) == 0)
{
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any text search configuration named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any text search configurations.");
- }
PQclear(res);
- return false;
+ if (pattern)
+ return listTSConfigs(pattern, false);
+ else
+ return true;
}
for (i = 0; i < PQntuples(res); i++)
@@ -5996,6 +6034,12 @@ listForeignDataWrappers(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -6072,6 +6116,12 @@ listForeignServers(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -6127,6 +6177,12 @@ listUserMappings(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -6199,6 +6255,12 @@ listForeignTables(const char *pattern, bool verbose)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -6253,6 +6315,12 @@ listExtensions(const char *pattern)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
}
@@ -6293,16 +6361,11 @@ listExtensionContents(const char *pattern)
if (PQntuples(res) == 0)
{
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any extension named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any extensions.");
- }
PQclear(res);
- return false;
+ if (pattern)
+ return listExtensions(pattern);
+ else
+ return true;
}
for (i = 0; i < PQntuples(res); i++)
@@ -6510,6 +6573,12 @@ listPublications(const char *pattern)
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if (pattern && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
@@ -6660,18 +6729,12 @@ describePublications(const char *pattern)
if (PQntuples(res) == 0)
{
- if (!pset.quiet)
- {
- if (pattern)
- pg_log_error("Did not find any publication named \"%s\".",
- pattern);
- else
- pg_log_error("Did not find any publications.");
- }
-
termPQExpBuffer(&buf);
PQclear(res);
- return false;
+ if (pattern)
+ return listPublications(pattern);
+ else
+ return true;
}
for (i = 0; i < PQntuples(res); i++)
@@ -7051,6 +7114,12 @@ listOperatorClasses(const char *access_method_pattern,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if ((access_method_pattern || type_pattern) && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
@@ -7139,6 +7208,12 @@ listOperatorFamilies(const char *access_method_pattern,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if ((access_method_pattern || type_pattern) && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
@@ -7246,6 +7321,12 @@ listOpFamilyOperators(const char *access_method_pattern,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if ((access_method_pattern || family_pattern) && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
@@ -7338,6 +7419,12 @@ listOpFamilyFunctions(const char *access_method_pattern,
printQuery(res, &myopt, pset.queryFout, false, pset.logfile);
+ if ((access_method_pattern || family_pattern) && PQntuples(res) == 0)
+ {
+ PQclear(res);
+ return false;
+ }
+
PQclear(res);
return true;
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index cf07a9dbd5e..011a6508df0 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -537,4 +537,30 @@ psql_fails_like(
qr/backslash commands are restricted; only \\unrestrict is allowed/,
'meta-command in restrict mode fails');
+# Test that \d commands return exit code 1 when pattern matches nothing
+# and don't print "Did not find" error messages
+{
+ my ($ret, $stdout, $stderr);
+
+ ($ret, $stdout, $stderr) = $node->psql('postgres', '\\d nonexistent_table');
+ isnt($ret, 0, '\\d with non-existent pattern: exit code not 0');
+ unlike($stderr, qr/Did not find/, '\\d with non-existent pattern: no error message');
+
+ ($ret, $stdout, $stderr) = $node->psql('postgres', '\\dt nonexistent_table');
+ isnt($ret, 0, '\\dt with non-existent pattern: exit code not 0');
+ unlike($stderr, qr/Did not find/, '\\dt with non-existent pattern: no error message');
+
+ ($ret, $stdout, $stderr) = $node->psql('postgres', '\\df nonexistent_func');
+ isnt($ret, 0, '\\df with non-existent pattern: exit code not 0');
+ unlike($stderr, qr/Did not find/, '\\df with non-existent pattern: no error message');
+
+ ($ret, $stdout, $stderr) = $node->psql('postgres', '\\dx+ nonexistent_ext');
+ isnt($ret, 0, '\\dx+ with non-existent pattern: exit code not 0');
+ unlike($stderr, qr/Did not find/, '\\dx+ with non-existent pattern: no error message');
+
+ # Test that \d commands without patterns succeed even with no results
+ ($ret, $stdout, $stderr) = $node->psql('postgres', '\\d');
+ is($ret, 0, '\\d without pattern: exit code 0');
+}
+
done_testing();
--
2.50.1
On Wed, Oct 15, 2025 at 10:56:47PM -0500, Abhishek Chanda wrote:
Thanks a lot for the review, Robin. And I am terribly sorry for the
slow reply, it just took me a while to get to this. But I think I have
addressed all your feedback, I have changed everything to set an error
code of 1 if anything is not empty. Also added tests for the return
code as requested, and cleaned up the change in describeRoles().
Please let me know if I missed anything.Attached an updated and rebased version of the patch.
There is something that I am not following here. The consensus of the
thread seems to be that folks are OK to show the results as empty
tables instead of an error, backtracking on the "historical" reasons
documented in the code. That's OK by me. Why not.
However, your patch does the opposite. For example:
=# \dg "no.such.role"
Did not find any role named ""no.such.role"".
On HEAD, this returns:
=# \dg "no.such.role"
List of roles
Role name | Attributes
-----------+------------
So your patch is doing the opposite of the consensus I am reading.
Note that `make check` complains immediately with that. You also may
want to be careful that all the code paths you are patching are
covered in the regression tests.
--
Michael