[ https://issues.apache.org/jira/browse/CALCITE-4621?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17352671#comment-17352671 ]
Julian Hyde commented on CALCITE-4621: -------------------------------------- The change to {{SemiJoinRule}} looks fine, but I am concerned about your change to {{RelOptRulesTest}}. Because you call {{assertEquals}} rather than {{diffRepos.assertEquals}}, if the plan is out of date, it will not generate a revised {{RelOptRules_actual.xml}}. Of secondary concern: there are now quite a few tests that don't use SQL and we don't have a concise way of writing these tests. Each such test has quite a few lines of boilerplate that seem to be copy-pasted from one test to the next. I think we can do better. [relFn|https://github.com/apache/calcite/blob/204b5ab42d9e365c55636cd0aca9f750f4d50e5d/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java#L120] is a pattern used successfully in several tests there the test provides a callback that, given a RelBuilder, generates a RelNode. In particular, the helper method {{private void testSwapJoinShouldNotMatch(RelNode input)}} you added for CALCITE-4042 (and similar code in this latest change). At a minimum, I would change its prefix "test" to "check" ("test" is reserved for actual tests, which this is not, because it has an argument). But I would change its argument type from {{RelNode}} to {{Function<RelBuilder, RelNode> relFn}}. That sets on a path towards re-using test code. > SemiJoinRule throws AssertionError on ANTI join > ----------------------------------------------- > > Key: CALCITE-4621 > URL: https://issues.apache.org/jira/browse/CALCITE-4621 > Project: Calcite > Issue Type: Bug > Components: core > Affects Versions: 1.26.0 > Reporter: Ruben Q L > Assignee: Ruben Q L > Priority: Major > Labels: pull-request-available > Fix For: 1.27.0 > > Time Spent: 1h 10m > Remaining Estimate: 0h > > Credits to [~thomas.rebele] for discovering the issue. > When SemiJoinRule (both {{CoreRules.JOIN_TO_SEMI_JOIN}} and > {{CoreRules.PROJECT_TO_SEMI_JOIN}}) matches an ANTI join, it fails with the > following error: > {noformat} > java.lang.AssertionError: ANTI > at org.apache.calcite.rel.rules.SemiJoinRule.perform(SemiJoinRule.java:122) > ... > {noformat} > The problem is that the rule config only forbids RIGHT and FULL joins, and > lets ANTI go through. Later when the rule is actually executed, joins with > type ANTI cannot be handled, hence the {{AssertionError}}. > The rule config must be adapted to forbid also ANTI joins. -- This message was sent by Atlassian Jira (v8.3.4#803005)