patch: improve "user mapping not found" error message

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

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.