Hi Japin Li
Thank you for your reviewing ,This way the notes are more accurate and complete. Thanks also to the author for updating the patch ,I also tested the new patch ,It looks good to me
Regrads
On Mon, 24 Jun 2024 at 17:59, Richard Guo <guofenglinux@gmail.com> wrote:
> Thank you for reviewing.
>
> On Mon, Jun 24, 2024 at 1:27 PM Li Japin <japinli@hotmail.com> wrote:
>> + /*
>> + * For now we do not support RIGHT_SEMI join in mergejoin or nestloop
>> + * join.
>> + */
>> + if (jointype == JOIN_RIGHT_SEMI)
>> + return;
>> +
>>
>> How about adding some reasons here?
>
> I've included a brief explanation in select_mergejoin_clauses.
>
Thank you for updating the patch.
>> + * this is a right-semi join, or this is a right/right-anti/full join and
>> + * there are nonmergejoinable join clauses. The executor's mergejoin
>>
>> Maybe we can put the right-semi join together with the right/right-anti/full
>> join. Is there any other significance by putting it separately?
>
> I don't think so. The logic is different: for right-semi join we will
> always set *mergejoin_allowed to false, while for right/right-anti/full
> join it is set to false only if there are nonmergejoinable join clauses.
>
Got it. Thanks for the explanation.
>> Maybe the following comments also should be updated. Right?
>
> Correct. And there are a few more places where we need to mention
> JOIN_RIGHT_SEMI, such as in reduce_outer_joins_pass2 and in the comment
> for SpecialJoinInfo.
>
>
> I noticed that this patch changes the plan of a query in join.sql from
> a semi join to right semi join, compromising the original purpose of
> this query, which was to test the fix for neqjoinsel's behavior for
> semijoins (see commit 7ca25b7d).
>
> --
> -- semijoin selectivity for <>
> --
> explain (costs off)
> select * from int4_tbl i4, tenk1 a
> where exists(select * from tenk1 b
> where a.twothousand = b.twothousand and a.fivethous <> b.fivethous)
> and i4.f1 = a.tenthous;
>
> So I've changed this test case a bit so that it is still testing what it
> is supposed to test.
>
> In passing, I've also updated the commit message to clarify that this
> patch does not address the support of "Right Semi Join" for merge joins.
>
Tested and looks good to me!
--
Regrads,
Japin Li
