public inbox for [email protected]help / color / mirror / Atom feed
Re: are the 2 if-statements in join_is_legal() removable? 3+ messages / 2 participants [nested] [flat]
* Re: are the 2 if-statements in join_is_legal() removable? @ 2025-05-10 15:47 Tom Lane <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Tom Lane @ 2025-05-10 15:47 UTC (permalink / raw) To: g l <[email protected]>; +Cc: pgsql-general g l <[email protected]> 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 ^ permalink raw reply [nested|flat] 3+ messages in thread
* 回复: are the 2 if-statements in join_is_legal() removable? @ 2025-05-11 08:26 g l <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: g l @ 2025-05-11 08:26 UTC (permalink / raw) To: pgsql-general 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 protected]> 发送时间: 2025年5月10日 18:47 收件人: g l <[email protected]> 抄送: [email protected] <[email protected]> 主题: Re: are the 2 if-statements in join_is_legal() removable? g l <[email protected]> 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 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: 回复: are the 2 if-statements in join_is_legal() removable? @ 2025-05-11 15:31 Tom Lane <[email protected]> parent: g l <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tom Lane @ 2025-05-11 15:31 UTC (permalink / raw) To: g l <[email protected]>; +Cc: pgsql-general g l <[email protected]> writes: > 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. 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 ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2025-05-11 15:31 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-05-10 15:47 Re: are the 2 if-statements in join_is_legal() removable? Tom Lane <[email protected]> 2025-05-11 08:26 ` 回复: are the 2 if-statements in join_is_legal() removable? g l <[email protected]> 2025-05-11 15:31 ` Re: 回复: are the 2 if-statements in join_is_legal() removable? Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox