On Thu, Jan 21, 2021 at 11:15 AM Hou, Zhijie <houzj.f...@cn.fujitsu.com> wrote:
>
> > Attaching v15 patch set. Please consider it for further review.
>
> Hi
>
> I have some comments for the 0001 patch
>
> In v15-0001-postgres_fdw-function-to-discard-cached-connecti
>
> 1.
> + If there is no open connection to the given foreign server,
> <literal>false</literal>
> + is returned. If no foreign server with the given name is found, an
> error
>
> Do you think it's better add some testcases about:
> call postgres_fdw_disconnect and postgres_fdw_disconnect_all when
> there is no open connection to the given foreign server
Do you mean a test case where foreign server exists but
postgres_fdw_disconnect() returns false because there's no connection
for that server?
> 2.
> + /*
> + * For the given server, if we closed connection or
> it is still in
> + * use, then no need of scanning the cache further.
> + */
> + if (entry->server_hashvalue == hashvalue &&
> + (entry->xact_depth > 0 || result))
> + {
> + hash_seq_term(&scan);
> + break;
> + }
>
> If I am not wrong, is the following condition always true ?
> (entry->xact_depth > 0 || result)
It's not always true. But it seems like it's too confusing, please
have a look at the upthread suggestion to change this with
can_terminate_scan boolean.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
