[PATCH] polish the error message of creating proc
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t46651psql -h localhost -U postgresBuilt from patchset v1 (message #1), September 20, 2026 at 05:39 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t46651_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t46651_1 && git checkout t46651_1Patchset v1 (message #1) is on t46651_1
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
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]: /messages/by-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] /messages/by-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]https://www.postgresql.org/docs/devel/nls-programmer.html#NLS-GUIDELINES.
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