Dear Osumi-san,
Thanks for reviewing! PSA v17 patchset.
> (1) v16-0001 : definition of a new structure
>
> CheckingRemoteServersCallbackItem can be added to typedefs.list.
Added.
> (2) v16-0001 : UnRegisterCheckingRemoteServersCallback's header comment
>
> +void
> +UnRegisterCheckingRemoteServersCallback(CheckingRemoteServersCallback
> callback,
> +
> void *arg)
> +{
>
> Looks require a header comment for this function,
> because in this file, all other functions have each one.
Added. Additionally, I renamed this function to Unregister...,
this follows other unregister functions.
> (3) v16-0002 : better place to declare new variables
>
> New variables 'old' and 'server' in GetConnection() can be moved to
> a scope of 'if' statement where those are used.
>
> @@ -138,6 +149,8 @@ GetConnection(UserMapping *user, bool will_prep_stmt,
> PgFdwConnState **state)
> ConnCacheEntry *entry;
> ConnCacheKey key;
> MemoryContext ccxt = CurrentMemoryContext;
> + MemoryContext old;
> + ForeignServer *server = GetForeignServer(user->serverid);
Fixed.
> (4) v16-0002 : typo in pgfdw_connection_check_internal()
>
>
> + /* return false is if the socket seems to be closed. */
>
> It should be "return false if the socket seems to be closed" ?
Fixed.
> (5) v16-0002 : pgfdw_connection_check's message
>
> When I tested the new feature, I got a below message.
>
> "ERROR: Foreign Server my_external_server might be down."
>
> I think we can wrap the server name with double quotes
> so that the message is more aligned with existing error message
> like connect_pg_server.
Fixed.
...I'm not sure whether this message is still need, because
the disconnection was delegated to XactCallback, and the function did following output:
```
WARNING: FATAL: terminating connection due to administrator command
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
```
> (6) v16-003 : typo
>
> + Authors needs to register the callback function via following API.
>
> Should be "Authors need to register the ...".
Fixed.
> (7) v16-0004 : unnecessary file
>
> I think we can remove a new file which looks like a log file.
>
> .../postgres_fdw/expected/postgres_fdw_1.out
This is needed to pass the test on the windows platform. See:
https://www.postgresql.org/docs/devel/regress-variant.html
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
