* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
> Brightwell, Adam wrote:
> > > If we were to make it consistent and use the old wording, what do you
> > > think about providing an "errhint" as well?
> > >
> > > Perhaps for example in slotfuncs.c#pg_create_physical_replication_stot:
> > >
> > > errmsg - "permission denied to create physical replication slot"
> > > errhint - "You must be superuser or replication role to use replication slots."
>
> Sure.
Sounds reasonable to me and looks to be in-line with wording elsewhere.
> > As I started looking at this, there are multiple other places where
> > these types of error messages occur (opclasscmds.c, user.c,
> > postinit.c, miscinit.c are just a few), not just around the changes in
> > this patch. If we change them in one place, wouldn't it be best to
> > change them in the rest? If that is the case, I'm afraid that might
> > distract from the purpose of this patch. Perhaps, if we want to
> > change them, then that should be submitted as a separate patch?
>
> Yeah. I'm just saying that maybe this patch should adopt whatever
> wording we agree to, not that we need to change other places. On the
> other hand, since so many other places have adopted the different
> wording, maybe there's a reason for it and if so, does anybody know what
> it is. But I have to say that it does look inconsistent to me.
I agree that this patch should adopt the wording agreed to above and
that it's 'more similar' to most of the usage in the backend. As for
the general question, I'd suggest we add to the error message policy
that more-or-less anything with errcode(ERRCODE_INSUFFICIENT_PRIVILEGE)
have errmsg("permission denied to X") with any comments about 'must be a
superuser' or similar relegated to errhint(). We could add that as a
TODO and have more junior developers review these cases and come up with
patches (or argue for cases where they feel deviation is warranted).
Either way, that larger rework isn't for this patch and since you seem
happy with it (modulo the change above), I'm going to make that change,
do my own review, and move foward with it unless someone else wants to
speak up.
Thanks!
Stephen