Thread: Adding error messages to a few slash commands

Adding error messages to a few slash commands

From
Abhishek Chanda
Date:
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

Attachment

Re: Adding error messages to a few slash commands

From
Abhishek Chanda
Date:
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



Re: Adding error messages to a few slash commands

From
Tom Lane
Date:
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



Re: Adding error messages to a few slash commands

From
Pavel Luzanov
Date:
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

Re: Adding error messages to a few slash commands

From
Abhishek Chanda
Date:
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

Attachment

Re: Adding error messages to a few slash commands

From
Pavel Luzanov
Date:
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

Re: Adding error messages to a few slash commands

From
Abhishek Chanda
Date:
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