Hi, This patch introduces new function postgres_fdw_disconnect() when called with a foreign server name discards the associated connections with the server name.
I think the following would read better: This patch introduces *a* new function postgres_fdw_disconnect(). When called with a foreign server name, it discards the associated connections with the server. Please note the removal of the 'name' at the end - connection is with server, not server name. + if (is_in_use) + ereport(WARNING, + (errmsg("cannot close the connection because it is still in use"))); It would be better to include servername in the message. + ereport(WARNING, + (errmsg("cannot close all connections because some of them are still in use"))); I think showing the number of active connections would be more informative. This can be achieved by changing active_conn_exists from bool to int (named active_conns, e.g.): + if (entry->conn && !active_conn_exists) + active_conn_exists = true; Instead of setting the bool value, active_conns can be incremented. Cheers On Sat, Jan 16, 2021 at 11:39 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > On Sat, Jan 16, 2021 at 10:36 AM Bharath Rupireddy > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > Please consider the v9 patch set for further review. > > > > > > Thanks for updating the patch! I reviewed only 0001 patch. > > I addressed the review comments and attached v10 patch set. Please > consider it for further review. > > > With Regards, > Bharath Rupireddy. > EnterpriseDB: http://www.enterprisedb.com >