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