On 2021/01/21 14:46, Bharath Rupireddy wrote:
On Thu, Jan 21, 2021 at 10:06 AM Fujii Masao
<masao.fu...@oss.nttdata.com> wrote:
> >> + if (entry->server_hashvalue == hashvalue &&
+ (entry->xact_depth > 0 || result))
+ {
+ hash_seq_term(&scan);
+ break;
entry->server_hashvalue can be 0? If yes, since postgres_fdw_disconnect_all()
specifies 0 as hashvalue, ISTM that the above condition can be true
unexpectedly. Can we replace this condition with just "if (!all)"?
I don't think so entry->server_hashvalue can be zero, because
GetSysCacheHashValue1/CatalogCacheComputeHashValue will not return 0
as hash value. I have not seen someone comparing hashvalue with an
expectation that it has 0 value, for instance see if (hashvalue == 0
|| riinfo->oidHashValue == hashvalue).
Having if(!all) something like below there doesn't suffice because we
might call hash_seq_term, when some connection other than the given
foreign server connection is in use.
No because we check the following condition before reaching that code. No?
+ if ((all || entry->server_hashvalue == hashvalue) &&
I was thinking that "(entry->xact_depth > 0 || result))" condition is not
necessary because "result" is set to true when xact_depth <= 0 and that
condition always indicates true.
I think that condition is too confusing. How about having a boolean
can_terminate_scan like below?
Thanks for thinking this. But at least for me, "if (!all)" looks not so
confusing.
And the comment seems to explain why we can end the scan.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
