Обсуждение: patch: improve "user mapping not found" error message

Поиск
Список
Период
Сортировка

patch: improve "user mapping not found" error message

От
Ian Lawrence Barwick
Дата:
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

Вложения

Re: patch: improve "user mapping not found" error message

От
Laurenz Albe
Дата:
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



Re: patch: improve "user mapping not found" error message

От
Peter Eisentraut
Дата:
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().



Re: patch: improve "user mapping not found" error message

От
Ian Lawrence Barwick
Дата:
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

Вложения

Re: patch: improve "user mapping not found" error message

От
Peter Eisentraut
Дата:
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.




Re: patch: improve "user mapping not found" error message

От
Peter Eisentraut
Дата:
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.