Dear Fujii-san,
> Thanks for updating the patch!
>
> > - Changed the name of new API from `GetUserMappingFromOid` to
> `GetUserMappingByOid`
> > to keep the name consistent with others.
>
> If we expose this function as an FDW helper function, it should return
> a complete UserMapping object, including umoptions.
>
> However, if postgres_fdw_get_connections() is the only user of this function,
> I'm not inclined to expose it as an FDW helper.
One reason is that the function does not handle any specific data for postgres_fdw,
however, there are no users and requirests from other projects. Based on that, ok,
we can move it to connection.c. If needed, we can export it again.
> Instead, we can either get
> the user ID by user mapping OID directly in connection.c using SearchSysCache(),
> or add the user ID to ConnCacheEntry and use it in
> postgres_fdw_get_connections().
> Thought?
I moved the function to connection.c, which uses the SearchSysCache1().
I've tried both ways, and they worked well. One difference is that when we use
the extended ConnCacheEntry approach and the entry has been invalidated, we cannot
distinguish the reason. For example, in the below case, the entry is invalidated,
so the user_name of the output record will be NULL, whereas the user mapping is
actually still valid. We may be able to add the reason for invalidation, but
I'm not very motivated to modify the part.
```
BEGIN;
SELECT 1 FROM ft1 LIMIT 1; -- ft1 is at server "loopback"
...
ALTER SERVER loopback OPTIONS (ADD use_remote_estimate 'off');
...
SELECT * FROM postgres_fdw_get_connections() ORDER BY 1;
-> {"loopback", NULL, ....} will be returned
```
Another reason is that we can keep the code consistent with the server case.
The part does not read data from the entry, and we can follow.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
