[
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)
