Thread: 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
Attachment
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:
+1 for this patch.
+1
Returning empty tables is a more appropriate behavior.
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
Attachment
On 13.04.2025 19:40, Abhishek Chanda wrote:
Thanks.
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.
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 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