On Thu, Jan 21, 2021 at 12:17 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > 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.
May I know if it's okay to have the boolean can_terminate_scan as shown in [1]? [1] - https://www.postgresql.org/message-id/flat/CALj2ACVx0%2BiOsrAA-wXbo3RLAKqUoNvvEd7foJ0vLwOdu8XjXw%40mail.gmail.com With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com