On Mon, 24 Jun 2024 at 17:59, Richard Guo <guofengli...@gmail.com> wrote: > Thank you for reviewing. > > On Mon, Jun 24, 2024 at 1:27 PM Li Japin <japi...@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