Обсуждение: 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
Вложения
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.