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