Hi, Richard > On Apr 25, 2024, at 11:28, Richard Guo <guofengli...@gmail.com> wrote: > > Here is another rebase with a commit message to help review. I also > tweaked some comments.
Thank you for updating the patch, here are some comments on the v5 patch. + /* + * 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? + * 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? Maybe the following comments also should be updated. Right? diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 5482ab85a7..791cbc551e 100644 --- a/src/backend/optimizer/prep/prepjointree.c +++ b/src/backend/optimizer/prep/prepjointree.c @@ -455,8 +455,8 @@ pull_up_sublinks_jointree_recurse(PlannerInfo *root, Node *jtnode, * point of the available_rels machinations is to ensure that we only * pull up quals for which that's okay. * - * We don't expect to see any pre-existing JOIN_SEMI, JOIN_ANTI, or - * JOIN_RIGHT_ANTI jointypes here. + * We don't expect to see any pre-existing JOIN_SEMI, JOIN_ANTI, + * JOIN_RIGHT_SEMI, or JOIN_RIGHT_ANTI jointypes here. */ switch (j->jointype) { @@ -2951,6 +2951,7 @@ reduce_outer_joins_pass2(Node *jtnode, * so there's no way that upper quals could refer to their * righthand sides, and no point in checking. We don't expect * to see JOIN_RIGHT_ANTI yet. + * Does JOIN_RIGHT_SEMI is expected here? */ break; default: