mihaibudiu commented on code in PR #4361:
URL: https://github.com/apache/calcite/pull/4361#discussion_r2073997739
##########
core/src/main/java/org/apache/calcite/rel/rules/IntersectToSemiJoinRule.java:
##########
@@ -34,9 +34,52 @@
import java.util.List;
/**
- * Planner rule that translates a {@link org.apache.calcite.rel.core.Intersect}
+ * Planner rule that translates a {@link Intersect}
* to a series of {@link org.apache.calcite.rel.core.Join} that type is
- * {@link org.apache.calcite.rel.core.JoinRelType#SEMI}.
+ * {@link JoinRelType#SEMI}. This rule supports n-way Intersect conversion,
+ * as this rule can be repeatedly applied during query optimization to
+ * refine the plan.
+ *
+ * <h2>Example</h2>
+ *
+ <p>Original sql:
Review Comment:
Is this the most general example for this rule?
If so, please explain in the comments the constraints on the supported
predicates - e.g., they all have to involve the same field.
If not, please provide a more general example where the predicates look very
different
##########
core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java:
##########
@@ -3633,6 +3633,19 @@ private void
checkPushJoinThroughUnionOnRightDoesNotMatchSemiOrAntiJoin(JoinRelT
.check();
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-7000";>[CALCITE-7000]
+ * Extend IntersectToSemiJoinRule to support n-way inputs</a>. */
+ @Test void testIntersectToSemiJoin2() {
Review Comment:
I am not sure whether union inserts properly explicit casts on the columns
that are unioned.
Can you try an example where the union is done on two collections with
different column types, so we can see whether the join conditions look correct
(they *will* require explicit casts).
##########
core/src/test/resources/sql/planner.iq:
##########
@@ -128,7 +128,60 @@ EnumerableCalc(expr#0..2=[{inputs}], $f0=[$t1], $f1=[$t2])
EnumerableCalc(expr#0=[{inputs}], expr#1=[IS NOT NULL($t0)],
DEPTNO=[$t0], $condition=[$t1])
EnumerableValues(tuples=[[{ 10 }, { 10 }, { 20 }, { 30 }, { 30 }, {
50 }, { 50 }, { 60 }, { null }]])
!plan
+!set planner-rules original
+
Review Comment:
please add some nulls in the tests too, especially in the columns that are
joined on.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
