Re: Adding error messages to a few slash commands - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Adding error messages to a few slash commands
Date
Msg-id aPBye_cT_wDjb6-L@paquier.xyz
Whole thread Raw
In response to Re: Adding error messages to a few slash commands  (Abhishek Chanda <abhishek.becs@gmail.com>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: [PROPOSAL] comments in repl_scanner
Next
From: Chao Li
Date:
Subject: Re: Checkpointer write combining