Subject: Re: are the 2 if-statements in join_is_legal() removable?

From: Tom Lane <[email-masked]>
Date: Sat, 10 May 2025 11:47:24 -0400

g l <[email-masked]> writes:
> In join_is_legal(), there are 2 decision-making statements based on match_sjinfo. I    wonder wether their conditions can ever test possitive.

The code coverage report says yes.

https://coverage.postgresql.org/src/backend/optimizer/path/joinrels.c.gcov.html#350

			regards, tom lane

---

From: g l <[email-masked]>
Date: Sun, 11 May 2025 08:26:38 +0000

Hi Tom:
Thanks for your reply! I add  logs like this:
            if (bms_is_subset(sjinfo->min_lefthand, rel1->relids) &&
                  bms_is_subset(sjinfo->min_righthand, rel2->relids))
            {
                  if (match_sjinfo) {
                        elog(LOG, "gunkris111111111111");
                        return false;     /* invalid join path */
                  }
                  match_sjinfo = sjinfo;
                  reversed = false;
            }
            else if (bms_is_subset(sjinfo->min_lefthand, rel2->relids) &&
                         bms_is_subset(sjinfo->min_righthand, rel1->relids))
            {
                  if (match_sjinfo) {
                        elog(LOG, "gunkris222222222222");
                        return false;     /* invalid join path */
                  }
                  match_sjinfo = sjinfo;
                  reversed = true;
            }
            else if (sjinfo->jointype == JOIN_SEMI &&
                         bms_equal(sjinfo->syn_righthand, rel2->relids) &&
                         create_unique_path(root, rel2, rel2->cheapest_total_path,
                                                      sjinfo) != NULL)
            {

                  if (match_sjinfo) {
                        elog(LOG, "gunkris33333333333333");
                        return false;     /* invalid join path */
                  }
                  match_sjinfo = sjinfo;
                  reversed = false;
                  unique_ified = true;
            }
            else if (sjinfo->jointype == JOIN_SEMI &&
                         bms_equal(sjinfo->syn_righthand, rel1->relids) &&
                         create_unique_path(root, rel1, rel1->cheapest_total_path,
                                                      sjinfo) != NULL)
            {
                  /* Reversed semijoin case */
                  if (match_sjinfo) {
                        elog(LOG, "gunkris4444444444444444");
                        return false;     /* invalid join path */
                  }
                  match_sjinfo = sjinfo;
                  reversed = true;
                  unique_ified = true;
            }
Then I run the core regression tests with "make installcheck-parallel
" for v18beta1 as the document dictates. When the regression test is done, only the last 2 log entries are found in logfile, the first 2 entries are not printed at all. I run core regression also with v14.15 and v17.2, the results are the same. What test suite do I need to run to cover the first 2 branches, or could you please show me a example query that can cover them? Thank you very much.

________________________________
发件人: Tom Lane <[email-masked]>
发送时间: 2025年5月10日 18:47
收件人: g l <[email-masked]>
抄送: [email-masked] <[email-masked]>
主题: Re: are the 2 if-statements in join_is_legal() removable?

                        regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Sun, 11 May 2025 11:31:39 -0400

> " for v18beta1 as the document dictates. When the regression test is done, only the last 2 log entries are found in logfile, the first 2 entries are not printed at all. I run core regression also w…

The coverage.postgresql.org results are, I believe, for check-world
not just the core tests.  You might try contrib/postgres_fdw first,
as that tends to be the non-core test with the most planner coverage.

In any case, the fact that a few of these lines are not reached by
test cases doesn't constitute a bug, and it most certainly doesn't
mean it'd be safe to remove them.  Most of the logic in join_is_legal
comes in symmetrical pairs of cases for the two possible orderings
of the two input relations.  It's just coincidental which of a pair
of cases will be exercised by a given phrasing of a SQL query.
So I don't feel that we're particularly short on coverage here:
one or the other code path is fully exercised in each case,
according to the coverage.postgresql.org results.

			regards, tom lane