Thread: [PATCH] polish the error message of creating proc
when a error occurs when creating proc, it should point out the
specific proc kind instead of just printing "function".
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index a9fe45e347..58af4b48ce 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -373,7 +373,11 @@ ProcedureCreate(const char *procedureName,
if (!replace)
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_FUNCTION),
- errmsg("function \"%s\"
already exists with same argument types",
+ errmsg("%s \"%s\" already
exists with same argument types",
+
(oldproc->prokind == PROKIND_AGGREGATE ? "aggregate function" :
+
oldproc->prokind == PROKIND_PROCEDURE ? "procedure" :
+
oldproc->prokind == PROKIND_WINDOW ? "window function" :
+ "function"),
procedureName)));
if (!pg_proc_ownercheck(oldproc->oid, proowner))
aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
--
Regards
Junwang Zhao
Attachment
Hi,
On Wed, Sep 21, 2022 at 07:45:01PM +0800, Junwang Zhao wrote:
> when a error occurs when creating proc, it should point out the
> specific proc kind instead of just printing "function".
You should have kept the original thread in copy (1), or at least mention it.
> diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
> index a9fe45e347..58af4b48ce 100644
> --- a/src/backend/catalog/pg_proc.c
> +++ b/src/backend/catalog/pg_proc.c
> @@ -373,7 +373,11 @@ ProcedureCreate(const char *procedureName,
> if (!replace)
> ereport(ERROR,
> (errcode(ERRCODE_DUPLICATE_FUNCTION),
> - errmsg("function \"%s\"
> already exists with same argument types",
> + errmsg("%s \"%s\" already
> exists with same argument types",
> +
> (oldproc->prokind == PROKIND_AGGREGATE ? "aggregate function" :
> +
> oldproc->prokind == PROKIND_PROCEDURE ? "procedure" :
> +
> oldproc->prokind == PROKIND_WINDOW ? "window function" :
> + "function"),
> procedureName)));
> if (!pg_proc_ownercheck(oldproc->oid, proowner))
> aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
You can't put part of the message in parameter, as the resulting string isn't
translatable. You should either use "routine" as a generic term or provide 3
different full messages.
[1] https://www.postgresql.org/message-id/29ea5666.6ce8.1835f4b4992.Coremail.qtds_126@126.com
On Wed, Sep 21, 2022 at 8:17 PM Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> Hi,
>
> On Wed, Sep 21, 2022 at 07:45:01PM +0800, Junwang Zhao wrote:
> > when a error occurs when creating proc, it should point out the
> > specific proc kind instead of just printing "function".
>
> You should have kept the original thread in copy (1), or at least mention it.
Yeah, thanks for pointing that out, will do that next time.
>
> > diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
> > index a9fe45e347..58af4b48ce 100644
> > --- a/src/backend/catalog/pg_proc.c
> > +++ b/src/backend/catalog/pg_proc.c
> > @@ -373,7 +373,11 @@ ProcedureCreate(const char *procedureName,
> > if (!replace)
> > ereport(ERROR,
> > (errcode(ERRCODE_DUPLICATE_FUNCTION),
> > - errmsg("function \"%s\"
> > already exists with same argument types",
> > + errmsg("%s \"%s\" already
> > exists with same argument types",
> > +
> > (oldproc->prokind == PROKIND_AGGREGATE ? "aggregate function" :
> > +
> > oldproc->prokind == PROKIND_PROCEDURE ? "procedure" :
> > +
> > oldproc->prokind == PROKIND_WINDOW ? "window function" :
> > + "function"),
> > procedureName)));
> > if (!pg_proc_ownercheck(oldproc->oid, proowner))
> > aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_FUNCTION,
>
> You can't put part of the message in parameter, as the resulting string isn't
> translatable. You should either use "routine" as a generic term or provide 3
> different full messages.
I noticed that there are some translations under the backend/po directory,
can we just change
msgid "function \"%s\" already exists with same argument types"
to
msgid "%s \"%s\" already exists with same argument types" ?
>
> [1] https://www.postgresql.org/message-id/29ea5666.6ce8.1835f4b4992.Coremail.qtds_126@126.com
--
Regards
Junwang Zhao
On Wed, Sep 21, 2022 at 10:35:47PM +0800, Junwang Zhao wrote: > On Wed, Sep 21, 2022 at 8:17 PM Julien Rouhaud <rjuju123@gmail.com> wrote: > > > > You can't put part of the message in parameter, as the resulting string isn't > > translatable. You should either use "routine" as a generic term or provide 3 > > different full messages. > > I noticed that there are some translations under the backend/po directory, > can we just change > msgid "function \"%s\" already exists with same argument types" > to > msgid "%s \"%s\" already exists with same argument types" ? The problem is that the parameters are passed as-is, so the final emitted translated string will be a mix of both languages, which isn't acceptable.
Junwang Zhao <zhjwpku@gmail.com> writes:
> I noticed that there are some translations under the backend/po directory,
> can we just change
> msgid "function \"%s\" already exists with same argument types"
> to
> msgid "%s \"%s\" already exists with same argument types" ?
No. This doesn't satisfy our message translation guidelines [1].
The fact that there are other messages that aren't up to project
standard isn't a license to create more.
More generally: there are probably dozens, if not hundreds, of
messages in the backend that say "function" but nowadays might
also be talking about a procedure. I'm not sure there's value
in improving just one of them.
I am pretty sure that we made an explicit decision some time back
that it is okay to say "function" when the object could also be
an aggregate or window function. So you could at least cut this
back to just handling "procedure" and "function". Or you could
change it to "routine" as Julien suggests, but I think a lot of
people will not think that's an improvement.
regards, tom lane
[1] https://www.postgresql.org/docs/devel/nls-programmer.html#NLS-GUIDELINES
On Wed, Sep 21, 2022 at 10:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Junwang Zhao <zhjwpku@gmail.com> writes: > > I noticed that there are some translations under the backend/po directory, > > can we just change > > msgid "function \"%s\" already exists with same argument types" > > to > > msgid "%s \"%s\" already exists with same argument types" ? > > No. This doesn't satisfy our message translation guidelines [1]. > The fact that there are other messages that aren't up to project > standard isn't a license to create more. > > More generally: there are probably dozens, if not hundreds, of > messages in the backend that say "function" but nowadays might > also be talking about a procedure. I'm not sure there's value > in improving just one of them. > > I am pretty sure that we made an explicit decision some time back > that it is okay to say "function" when the object could also be > an aggregate or window function. So you could at least cut this > back to just handling "procedure" and "function". Or you could > change it to "routine" as Julien suggests, but I think a lot of > people will not think that's an improvement. Yeah, make sense, will leave it as is. > > regards, tom lane > > [1] https://www.postgresql.org/docs/devel/nls-programmer.html#NLS-GUIDELINES -- Regards Junwang Zhao