On Fri, Jul 2, 2021 at 11:59 AM Zhihong Yu <z...@yugabyte.com> wrote:
> > > On Thu, Jul 1, 2021 at 8:24 PM Richard Guo <guofengli...@gmail.com> wrote: > >> >> On Thu, Jul 1, 2021 at 3:18 PM Ronan Dunklau <ronan.dunk...@aiven.io> >> wrote: >> >>> Le jeudi 1 juillet 2021, 09:09:38 CEST Ronan Dunklau a écrit : >>> > > Yes, thanks! I was making a big mistake here thinking the executor >>> can >>> > > stop after the first match. That's not true. We need to use each >>> outer >>> > > tuple to find all the matches and mark the corresponding hashtable >>> > > entries. I have updated the patch with the fix. >>> > >>> > It looks OK to me. >>> >>> I forgot to mention: you also have failing tests due to the plans >>> changing to >>> use the new join type. This might not be the case anymore once you >>> update the >>> cost model, but if that's the case the tests should be updated. >>> >> >> Thanks! Test cases are updated in v3 patch. Also merge join can do the >> 'right anti join' too in the same patch. >> >> Thanks again for reviewing this patch. >> >> Thanks >> Richard >> >> > Hi, > Minor comment: > + * In a right-antijoin, we never return a matched > tuple. > > matched tuple -> matching tuple > Thanks for reviewing. The comment for JOIN_ANTI in ExecHashJoinImpl/ExecMergeJoin is: "In an antijoin, we never return a matched tuple" So I think we'd better keep it consistent for JOIN_RIGHT_ANTI? Thanks Richard
