Fix a wrong errmsg in AlterRole()

Started by cca5507over 1 year ago6 messageshackers
Jump to latest
#1cca5507
cca5507@qq.com

Hi,

```
postgres=# create group g1;
CREATE ROLE
postgres=# create role r1 in group g1;
CREATE ROLE
postgres=# set session authorization r1;
SET
postgres=> alter group g1 drop user r1;
ERROR:  permission denied to alter role
DETAIL:  Only roles with the ADMIN option on role "g1" may add members.
```

The errmsg is "add members", but we are doing a drop.

Here is a small patch to fix it.

--
Regards,
ChangAo Chen

Attachments:

0001-Fix-a-wrong-errmsg-in-AlterRole.patchapplication/octet-stream; charset=ISO-8859-1; name=0001-Fix-a-wrong-errmsg-in-AlterRole.patchDownload+15-7
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: cca5507 (#1)
Re: Fix a wrong errmsg in AlterRole()

"=?ISO-8859-1?B?Y2NhNTUwNw==?=" <cca5507@qq.com> writes:

postgres=&gt; alter group g1 drop user r1;
ERROR: permission denied to alter role
DETAIL: Only roles with the ADMIN option on role "g1" may add members.

The errmsg is "add members", but we are doing a drop.
Here is a small patch to fix it.

Hmm. Seems simpler to just make the errdetail say "... may add or
drop members."

regards, tom lane

#3Nathan Bossart
nathandbossart@gmail.com
In reply to: Tom Lane (#2)
Re: Fix a wrong errmsg in AlterRole()

On Wed, Jan 08, 2025 at 10:29:12AM -0500, Tom Lane wrote:

"=?ISO-8859-1?B?Y2NhNTUwNw==?=" <cca5507@qq.com> writes:

postgres=&gt; alter group g1 drop user r1;
ERROR: permission denied to alter role
DETAIL: Only roles with the ADMIN option on role "g1" may add members.

The errmsg is "add members", but we are doing a drop.
Here is a small patch to fix it.

Hmm. Seems simpler to just make the errdetail say "... may add or
drop members."

+1. It looks like this goes back to commit ce6b672 (v16). The error
message was changed in commit de4d456, which is also a v16 change, so
fortunately this has a good chance of back-patching cleanly.

--
nathan

#4cca5507
cca5507@qq.com
In reply to: Nathan Bossart (#3)
Re: Fix a wrong errmsg in AlterRole()

Hi,

I modified the patch according to Tom's suggestion.

--
Regards,
ChangAo Chen

Attachments:

v2-0001-Fix-a-wrong-errdetail-in-AlterRole.patchapplication/octet-stream; charset=ISO-8859-1; name=v2-0001-Fix-a-wrong-errdetail-in-AlterRole.patchDownload+2-3
#5Nathan Bossart
nathandbossart@gmail.com
In reply to: cca5507 (#4)
Re: Fix a wrong errmsg in AlterRole()

On Thu, Jan 09, 2025 at 10:13:11AM +0800, cca5507 wrote:

I modified the patch according to Tom's suggestion.

LGTM. I'll plan on committing this shortly.

--
nathan

#6Nathan Bossart
nathandbossart@gmail.com
In reply to: Nathan Bossart (#5)
Re: Fix a wrong errmsg in AlterRole()

On Thu, Jan 09, 2025 at 12:27:20PM -0600, Nathan Bossart wrote:

LGTM. I'll plan on committing this shortly.

Committed.

--
nathan