patch: improve "user mapping not found" error message
Hi
Mild corner-case annoyance while doing Random Experimental Things:
postgres=# SELECT * FROM parttest;
ERROR: user mapping not found for "postgres"
Okaaaay, but which server?
postgres=# \det
List of foreign tables
Schema | Table | Server
--------+---------------+-----------
public | parttest_10_1 | fdw_node2
public | parttest_10_3 | fdw_node3
public | parttest_10_5 | fdw_node4
public | parttest_10_7 | fdw_node5
public | parttest_10_9 | fdw_node6
(5 rows)
(Muffled sound of small patch hatching) aha:
postgres=# SELECT * FROM parttest;
ERROR: user mapping not found for user "postgres", server "fdw_node5"
Regards
Ian Barwick
Attachments:
user-mapping-not-found-message-improvement.v1.patchtext/x-patch; charset=US-ASCII; name=user-mapping-not-found-message-improvement.v1.patchDownload
commit 1e08150c0bba813050b00e4e35cdba1572fd5564
Author: Ian Barwick <barwick@gmail.com>
Date: Fri Jun 23 16:36:41 2023 +0900
Improve "user mapping not found" error message
Display the name of the foreign server for which the user mapping
was not found.
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index ca3ad55b62..b649064141 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -217,10 +217,15 @@ GetUserMapping(Oid userid, Oid serverid)
}
if (!HeapTupleIsValid(tp))
+ {
+ ForeignServer *server = GetForeignServer(serverid);
+
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("user mapping not found for \"%s\"",
- MappingUserName(userid))));
+ errmsg("user mapping not found for user \"%s\", server \"%s\"",
+ MappingUserName(userid),
+ server->servername)));
+ }
um = (UserMapping *) palloc(sizeof(UserMapping));
um->umid = ((Form_pg_user_mapping) GETSTRUCT(tp))->oid;
On Fri, 2023-06-23 at 16:45 +0900, Ian Lawrence Barwick wrote:
Mild corner-case annoyance while doing Random Experimental Things:
postgres=# SELECT * FROM parttest;
ERROR: user mapping not found for "postgres"Okaaaay, but which server?
postgres=# \det
List of foreign tables
Schema | Table | Server
--------+---------------+-----------
public | parttest_10_1 | fdw_node2
public | parttest_10_3 | fdw_node3
public | parttest_10_5 | fdw_node4
public | parttest_10_7 | fdw_node5
public | parttest_10_9 | fdw_node6
(5 rows)(Muffled sound of small patch hatching) aha:
postgres=# SELECT * FROM parttest;
ERROR: user mapping not found for user "postgres", server "fdw_node5"
+1
Yours,
Laurenz Albe
On 23.06.23 09:45, Ian Lawrence Barwick wrote:
if (!HeapTupleIsValid(tp)) + { + ForeignServer *server = GetForeignServer(serverid); + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("user mapping not found for \"%s\"", - MappingUserName(userid)))); + errmsg("user mapping not found for user \"%s\", server \"%s\"", + MappingUserName(userid), + server->servername))); + }
What if the foreign server does not exist either? Then this would show
a "cache lookup failed" error message, which I think we should avoid.
There is existing logic for handling this in
get_object_address_usermapping().
2023年7月3日(月) 18:22 Peter Eisentraut <peter@eisentraut.org>:
On 23.06.23 09:45, Ian Lawrence Barwick wrote:
if (!HeapTupleIsValid(tp)) + { + ForeignServer *server = GetForeignServer(serverid); + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("user mapping not found for \"%s\"", - MappingUserName(userid)))); + errmsg("user mapping not found for user \"%s\", server \"%s\"", + MappingUserName(userid), + server->servername))); + }What if the foreign server does not exist either? Then this would show
a "cache lookup failed" error message, which I think we should avoid.There is existing logic for handling this in
get_object_address_usermapping().
Apologies, missed this response somewhere. Does the attached fix that?
Regards
Ian Barwick
Attachments:
user-mapping-not-found-message-improvement.v2.patchtext/x-patch; charset=US-ASCII; name=user-mapping-not-found-message-improvement.v2.patchDownload
commit 9b01a96a5a4fb816668c86c254b67dbf1083e4d5
Author: Ian Barwick <barwick@gmail.com>
Date: Sun Jul 23 05:49:13 2023 +0900
Improve "user mapping not found" error message
Display the name of the foreign server for which the user mapping
was not found.
diff --git a/src/backend/foreign/foreign.c b/src/backend/foreign/foreign.c
index ca3ad55b62..3432eb2841 100644
--- a/src/backend/foreign/foreign.c
+++ b/src/backend/foreign/foreign.c
@@ -217,10 +217,22 @@ GetUserMapping(Oid userid, Oid serverid)
}
if (!HeapTupleIsValid(tp))
+ {
+ ForeignServer *server = GetForeignServerExtended(serverid,
+ FSV_MISSING_OK);
+
+ if (!server)
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_OBJECT),
+ errmsg("user mapping not found for user \"%s\"",
+ MappingUserName(userid))));
+
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
- errmsg("user mapping not found for \"%s\"",
- MappingUserName(userid))));
+ errmsg("user mapping not found for user \"%s\", server \"%s\"",
+ MappingUserName(userid),
+ server->servername)));
+ }
um = (UserMapping *) palloc(sizeof(UserMapping));
um->umid = ((Form_pg_user_mapping) GETSTRUCT(tp))->oid;
On 20.11.23 02:25, Ian Lawrence Barwick wrote:
2023年7月3日(月) 18:22 Peter Eisentraut <peter@eisentraut.org>:
On 23.06.23 09:45, Ian Lawrence Barwick wrote:
if (!HeapTupleIsValid(tp)) + { + ForeignServer *server = GetForeignServer(serverid); + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("user mapping not found for \"%s\"", - MappingUserName(userid)))); + errmsg("user mapping not found for user \"%s\", server \"%s\"", + MappingUserName(userid), + server->servername))); + }What if the foreign server does not exist either? Then this would show
a "cache lookup failed" error message, which I think we should avoid.There is existing logic for handling this in
get_object_address_usermapping().Apologies, missed this response somewhere. Does the attached fix that?
Hmm, now that I look at this again, under what circumstances would the
server not be found? Maybe the first patch was right and it should give
a "scary" error in that case, instead of just omitting it.
In any case, this patch appears to be missing an update in the
postgres_fdw test output.
On 23.11.23 09:41, Peter Eisentraut wrote:
On 20.11.23 02:25, Ian Lawrence Barwick wrote:
2023年7月3日(月) 18:22 Peter Eisentraut <peter@eisentraut.org>:
On 23.06.23 09:45, Ian Lawrence Barwick wrote:
if (!HeapTupleIsValid(tp)) + { + ForeignServer *server = GetForeignServer(serverid); + ereport(ERROR, (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("user mapping not found for \"%s\"", - MappingUserName(userid)))); + errmsg("user mapping not found for user \"%s\", server \"%s\"", + MappingUserName(userid), + server->servername))); + }What if the foreign server does not exist either? Then this would show
a "cache lookup failed" error message, which I think we should avoid.There is existing logic for handling this in
get_object_address_usermapping().Apologies, missed this response somewhere. Does the attached fix that?
Hmm, now that I look at this again, under what circumstances would the
server not be found? Maybe the first patch was right and it should give
a "scary" error in that case, instead of just omitting it.In any case, this patch appears to be missing an update in the
postgres_fdw test output.
I have committed the first version of the patch together with the
required test changes.