Re: improving user.c error messages

While we're here,

On 2023-Jan-26, Nathan Bossart wrote:

> @@ -838,7 +867,8 @@ AlterRole(ParseState *pstate, AlterRoleStmt *stmt)
>          if (!should_be_super && roleid == BOOTSTRAP_SUPERUSERID)
>              ereport(ERROR,
>                      (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> -                     errmsg("permission denied: bootstrap user must be superuser")));
> +                     errmsg("permission denied to alter role"),
> +                     errdetail("The bootstrap user must be superuser.")));

I think this one isn't using the right errcode; this is not a case of
insufficient privileges.  There's no priv you can acquire that lets you
do it.  So I'd change it to unsupported operation.


I was confused a bit by this one:

>         /* an unprivileged user can change their own password */
>         if (dpassword && roleid != currentUserId)
>             ereport(ERROR,
>                     (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> -                    errmsg("must have CREATEROLE privilege to change another user's password")));
> +                    errmsg("permission denied to alter role"),
> +                    errdetail("To change another role's password, you must have %s privilege and %s on the role.",
> +                              "CREATEROLE", "ADMIN OPTION")));
>     }

In no other message we say what operation is being attempted in the
errdetail; all the others start with "You must have" and that's it.
However, looking closer I think this one being different is okay,
because the errmsg() you're using is vague, and I think the error report
would be confusing if you were to remove the "To change another role's
password" bit.

The patch looks good to me.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/