This is an automated email from the ASF dual-hosted git repository.
laurent pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/master by this push:
new c25b29c [CALCITE-2803] ProjectTransposeJoinRule messes INDF
expressions
c25b29c is described below
commit c25b29c0c1cae3a57bf91ddfd9f6a0622ca974a4
Author: Laurent Goujon <[email protected]>
AuthorDate: Wed Jan 23 14:32:00 2019 -0800
[CALCITE-2803] ProjectTransposeJoinRule messes INDF expressions
ProjectTransposeJoinRule does not identify expanded versions of IS NOT
DISTINCT FROM expressions in the join filter, and might push them below
the join operator in a way which makes impossible for
RelOptUtil#splitJoinCondition() to identify the INDF condition and mis-
categorize the join as not being an equi-join.
Fix the issue by collapsing INDF expressions before invoking PushProjector.
Also add support for expanded form CASE(WHEN A IS NULL THEN B IS NULL
WHEN B IS NULL THEN A IS NULL ELSE A = B).
---
.../java/org/apache/calcite/plan/RelOptUtil.java | 95 +++++++++++++++++++---
.../rel/rules/ProjectJoinTransposeRule.java | 24 +++++-
.../org/apache/calcite/plan/RelOptUtilTest.java | 54 +++++++++++-
.../org/apache/calcite/test/RelOptRulesTest.java | 10 +++
.../org/apache/calcite/test/RelOptRulesTest.xml | 23 ++++++
5 files changed, 190 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
index 809dd27..15733d9 100644
--- a/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
+++ b/core/src/main/java/org/apache/calcite/plan/RelOptUtil.java
@@ -1474,6 +1474,8 @@ public abstract class RelOptUtil {
}
/**
+ * Collapse an expanded version of IS NOT DISTINCT FROM expression
+ *
* Helper method for
* {@link #splitJoinCondition(RexBuilder, int, RexNode, List, List, List,
List)} and
* {@link #splitJoinCondition(List, List, RexNode, List, List, List, List)}.
@@ -1492,8 +1494,22 @@ public abstract class RelOptUtil {
* return a IS NOT DISTINCT FROM function call. Otherwise return the
input
* function call as it is.
*/
- private static RexCall collapseExpandedIsNotDistinctFromExpr(final RexCall
call,
+ public static RexCall collapseExpandedIsNotDistinctFromExpr(final RexCall
call,
final RexBuilder rexBuilder) {
+ switch (call.getKind()) {
+ case OR:
+ return doCollapseExpandedIsNotDistinctFromOrExpr(call, rexBuilder);
+
+ case CASE:
+ return doCollapseExpandedIsNotDistinctFromCaseExpr(call, rexBuilder);
+
+ default:
+ return call;
+ }
+ }
+
+ private static RexCall doCollapseExpandedIsNotDistinctFromOrExpr(final
RexCall call,
+ final RexBuilder rexBuilder) {
if (call.getKind() != SqlKind.OR || call.getOperands().size() != 2) {
return call;
}
@@ -1508,13 +1524,24 @@ public abstract class RelOptUtil {
RexCall opEqCall = (RexCall) op0;
RexCall opNullEqCall = (RexCall) op1;
+ // Swapping the operands if necessary
if (opEqCall.getKind() == SqlKind.AND
- && opNullEqCall.getKind() == SqlKind.EQUALS) {
+ && (opNullEqCall.getKind() == SqlKind.EQUALS
+ || opNullEqCall.getKind() == SqlKind.IS_TRUE)) {
RexCall temp = opEqCall;
opEqCall = opNullEqCall;
opNullEqCall = temp;
}
+ // Check if EQUALS is actually wrapped in IS TRUE expression
+ if (opEqCall.getKind() == SqlKind.IS_TRUE) {
+ RexNode tmp = opEqCall.getOperands().get(0);
+ if (!(tmp instanceof RexCall)) {
+ return call;
+ }
+ opEqCall = (RexCall) tmp;
+ }
+
if (opNullEqCall.getKind() != SqlKind.AND
|| opNullEqCall.getOperands().size() != 2
|| opEqCall.getKind() != SqlKind.EQUALS) {
@@ -1527,18 +1554,62 @@ public abstract class RelOptUtil {
|| op11.getKind() != SqlKind.IS_NULL) {
return call;
}
- final RexNode isNullInput0 = ((RexCall) op10).getOperands().get(0);
- final RexNode isNullInput1 = ((RexCall) op11).getOperands().get(0);
- final String isNullInput0Digest = isNullInput0.toString();
- final String isNullInput1Digest = isNullInput1.toString();
- final String equalsInput0Digest = opEqCall.getOperands().get(0).toString();
- final String equalsInput1Digest = opEqCall.getOperands().get(1).toString();
+ return doCollapseExpandedIsNotDistinctFrom(rexBuilder, call, (RexCall)
op10, (RexCall) op11,
+ opEqCall);
+ }
+
+ private static RexCall doCollapseExpandedIsNotDistinctFromCaseExpr(final
RexCall call,
+ final RexBuilder rexBuilder) {
+ if (call.getKind() != SqlKind.CASE || call.getOperands().size() != 5) {
+ return call;
+ }
+
+ final RexNode op0 = call.getOperands().get(0);
+ final RexNode op1 = call.getOperands().get(1);
+ final RexNode op2 = call.getOperands().get(2);
+ final RexNode op3 = call.getOperands().get(3);
+ final RexNode op4 = call.getOperands().get(4);
+
+ if (!(op0 instanceof RexCall) || !(op1 instanceof RexCall) || !(op2
instanceof RexCall)
+ || !(op3 instanceof RexCall) || !(op4 instanceof RexCall)) {
+ return call;
+ }
+
+ RexCall ifCall = (RexCall) op0;
+ RexCall thenCall = (RexCall) op1;
+ RexCall elseIfCall = (RexCall) op2;
+ RexCall elseIfThenCall = (RexCall) op3;
+ RexCall elseCall = (RexCall) op4;
+
+ if (ifCall.getKind() != SqlKind.IS_NULL
+ || thenCall.getKind() != SqlKind.IS_NULL
+ || elseIfCall.getKind() != SqlKind.IS_NULL
+ || elseIfThenCall.getKind() != SqlKind.IS_NULL
+ || elseCall.getKind() != SqlKind.EQUALS) {
+ return call;
+ }
+
+ if (!ifCall.equals(elseIfThenCall)
+ || !thenCall.equals(elseIfCall)) {
+ return call;
+ }
+
+ return doCollapseExpandedIsNotDistinctFrom(rexBuilder, call, ifCall,
elseIfCall, elseCall);
+ }
+
+ private static RexCall doCollapseExpandedIsNotDistinctFrom(final RexBuilder
rexBuilder,
+ final RexCall call, RexCall ifNull0Call, RexCall ifNull1Call, RexCall
equalsCall) {
+ final RexNode isNullInput0 = ifNull0Call.getOperands().get(0);
+ final RexNode isNullInput1 = ifNull1Call.getOperands().get(0);
+
+ final RexNode equalsInput0 = RexUtil
+ .removeNullabilityCast(rexBuilder.getTypeFactory(),
equalsCall.getOperands().get(0));
+ final RexNode equalsInput1 = RexUtil
+ .removeNullabilityCast(rexBuilder.getTypeFactory(),
equalsCall.getOperands().get(1));
- if ((isNullInput0Digest.equals(equalsInput0Digest)
- && isNullInput1Digest.equals(equalsInput1Digest))
- || (isNullInput1Digest.equals(equalsInput0Digest)
- && isNullInput0Digest.equals(equalsInput1Digest))) {
+ if ((isNullInput0.equals(equalsInput0) &&
isNullInput1.equals(equalsInput1))
+ || (isNullInput1.equals(equalsInput0) &&
isNullInput0.equals(equalsInput1))) {
return (RexCall)
rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_DISTINCT_FROM,
ImmutableList.of(isNullInput0, isNullInput1));
}
diff --git
a/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
b/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
index 52d22cc..feb01b6 100644
---
a/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
+++
b/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java
@@ -18,14 +18,17 @@ package org.apache.calcite.rel.rules;
import org.apache.calcite.plan.RelOptRule;
import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.rel.RelNode;
import org.apache.calcite.rel.core.Join;
import org.apache.calcite.rel.core.Project;
import org.apache.calcite.rel.core.RelFactories;
import org.apache.calcite.rel.core.SemiJoin;
import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rex.RexCall;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexOver;
+import org.apache.calcite.rex.RexShuttle;
import org.apache.calcite.tools.RelBuilderFactory;
import java.util.ArrayList;
@@ -77,6 +80,21 @@ public class ProjectJoinTransposeRule extends RelOptRule {
if (join instanceof SemiJoin) {
return; // TODO: support SemiJoin
}
+
+ // Normalize the join condition so we don't end up misidentified expanded
+ // form of IS NOT DISTINCT FROM as PushProject also visit the filter
condition
+ // and push down expressions.
+ RexNode joinFilter = join.getCondition().accept(new RexShuttle() {
+ @Override public RexNode visitCall(RexCall rexCall) {
+ final RexNode node = super.visitCall(rexCall);
+ if (!(node instanceof RexCall)) {
+ return node;
+ }
+ return RelOptUtil.collapseExpandedIsNotDistinctFromExpr((RexCall) node,
+ call.builder().getRexBuilder());
+ }
+ });
+
// locate all fields referenced in the projection and join condition;
// determine which inputs are referenced in the projection and
// join condition; if all fields are being referenced and there are no
@@ -84,7 +102,7 @@ public class ProjectJoinTransposeRule extends RelOptRule {
PushProjector pushProject =
new PushProjector(
origProj,
- join.getCondition(),
+ joinFilter,
join,
preserveExprCondition,
call.builder());
@@ -108,7 +126,7 @@ public class ProjectJoinTransposeRule extends RelOptRule {
// convert the join condition to reference the projected columns
RexNode newJoinFilter = null;
int[] adjustments = pushProject.getAdjustments();
- if (join.getCondition() != null) {
+ if (joinFilter != null) {
List<RelDataTypeField> projJoinFieldList = new ArrayList<>();
projJoinFieldList.addAll(
join.getSystemFieldList());
@@ -118,7 +136,7 @@ public class ProjectJoinTransposeRule extends RelOptRule {
rightProjRel.getRowType().getFieldList());
newJoinFilter =
pushProject.convertRefsAndExprs(
- join.getCondition(),
+ joinFilter,
projJoinFieldList,
adjustments);
}
diff --git a/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java
b/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java
index 664b418..2752034 100644
--- a/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java
+++ b/core/src/test/java/org/apache/calcite/plan/RelOptUtilTest.java
@@ -188,6 +188,58 @@ public class RelOptUtilTest {
REL_BUILDER.literal(true));
}
+ /**
+ * Test {@link RelOptUtil#splitJoinCondition(RelNode, RelNode, RexNode,
List, List, List)}
+ * where the join condition contains an expanded version of IS NOT DISTINCT
using CASE
+ */
+ @Test public void testSplitJoinConditionExpandedIsNotDistinctFromUsingCase()
{
+ int leftJoinIndex =
EMP_SCAN.getRowType().getFieldNames().indexOf("DEPTNO");
+ int rightJoinIndex = DEPT_ROW.getFieldNames().indexOf("DEPTNO");
+
+ RexInputRef leftKeyInputRef = RexInputRef.of(leftJoinIndex,
EMP_DEPT_JOIN_REL_FIELDS);
+ RexInputRef rightKeyInputRef =
+ RexInputRef.of(EMP_ROW.getFieldCount() + rightJoinIndex,
EMP_DEPT_JOIN_REL_FIELDS);
+ RexNode joinCond = RelOptUtil.isDistinctFrom(
+ REL_BUILDER.getRexBuilder(),
+ leftKeyInputRef,
+ rightKeyInputRef,
+ true);
+
+
+ splitJoinConditionHelper(
+ joinCond,
+ Collections.singletonList(leftJoinIndex),
+ Collections.singletonList(rightJoinIndex),
+ Collections.singletonList(false),
+ REL_BUILDER.literal(true));
+ }
+
+ /**
+ * Test {@link RelOptUtil#splitJoinCondition(RelNode, RelNode, RexNode,
List, List, List)}
+ * where the join condition contains an expanded version of IS NOT DISTINCT
using CASE
+ */
+ @Test public void
testSplitJoinConditionExpandedIsNotDistinctFromUsingCase2() {
+ int leftJoinIndex =
EMP_SCAN.getRowType().getFieldNames().indexOf("DEPTNO");
+ int rightJoinIndex = DEPT_ROW.getFieldNames().indexOf("DEPTNO");
+
+ RexInputRef leftKeyInputRef = RexInputRef.of(leftJoinIndex,
EMP_DEPT_JOIN_REL_FIELDS);
+ RexInputRef rightKeyInputRef =
+ RexInputRef.of(EMP_ROW.getFieldCount() + rightJoinIndex,
EMP_DEPT_JOIN_REL_FIELDS);
+ RexNode joinCond = REL_BUILDER.call(SqlStdOperatorTable.CASE,
+ REL_BUILDER.call(SqlStdOperatorTable.IS_NULL, leftKeyInputRef),
+ REL_BUILDER.call(SqlStdOperatorTable.IS_NULL, rightKeyInputRef),
+ REL_BUILDER.call(SqlStdOperatorTable.IS_NULL, rightKeyInputRef),
+ REL_BUILDER.call(SqlStdOperatorTable.IS_NULL, leftKeyInputRef),
+ REL_BUILDER.call(SqlStdOperatorTable.EQUALS, leftKeyInputRef,
rightKeyInputRef));
+
+ splitJoinConditionHelper(
+ joinCond,
+ Collections.singletonList(leftJoinIndex),
+ Collections.singletonList(rightJoinIndex),
+ Collections.singletonList(false),
+ REL_BUILDER.literal(true));
+ }
+
private static void splitJoinConditionHelper(RexNode joinCond, List<Integer>
expLeftKeys,
List<Integer> expRightKeys, List<Boolean> expFilterNulls, RexNode
expRemaining) {
List<Integer> actLeftKeys = new ArrayList<>();
@@ -197,7 +249,7 @@ public class RelOptUtilTest {
RexNode actRemaining = RelOptUtil.splitJoinCondition(EMP_SCAN, DEPT_SCAN,
joinCond, actLeftKeys,
actRightKeys, actFilterNulls);
- assertEquals(expRemaining.toString(), actRemaining.toString());
+ assertEquals(expRemaining, actRemaining);
assertEquals(expFilterNulls, actFilterNulls);
assertEquals(expLeftKeys, actLeftKeys);
assertEquals(expRightKeys, actRightKeys);
diff --git a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
index 2516f72..055af52 100644
--- a/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelOptRulesTest.java
@@ -5183,6 +5183,16 @@ public class RelOptRulesTest extends RelOptTestBase {
+ "from sales.dept group by name";
sql(sql).with(program).check();
}
+
+ /** Test case for
+ * <a href="https://issues.apache.org/jira/browse/CALCITE-2803";>[CALCITE-2803]
+ * Identify expanded IS NOT DISTINCT FROM expression when pushing project
past join</a>.
+ */
+ @Test public void testPushProjectWithIsNotDistinctFromPastJoin() {
+ checkPlanning(ProjectJoinTransposeRule.INSTANCE,
+ "select e.sal + b.comm from emp e inner join bonus b "
+ + "on e.ename IS NOT DISTINCT FROM b.ename and e.deptno = 10");
+ }
}
// End RelOptRulesTest.java
diff --git
a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
index e90f1b0..ad9d200 100644
--- a/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/RelOptRulesTest.xml
@@ -10139,4 +10139,27 @@ LogicalProject(NAME=[$0], EXPR$1=[CAST(POWER(/(-($1,
/(*($2, $2), $3)), $3), 0.5
]]>
</Resource>
</TestCase>
+ <TestCase name="testPushProjectWithIsNotDistinctFromPastJoin">
+ <Resource name="sql">
+ <![CDATA[select e.sal + b.comm from emp e inner join bonus b on
e.ename IS NOT DISTINCT FROM b.ename]]>
+ </Resource>
+ <Resource name="planBefore">
+ <![CDATA[
+LogicalProject(EXPR$0=[+($5, $12)])
+ LogicalJoin(condition=[AND(OR(AND(IS NULL($1), IS NULL($9)), IS TRUE(=($1,
$9))), =($7, 10))], joinType=[inner])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalTableScan(table=[[CATALOG, SALES, BONUS]])
+]]>
+ </Resource>
+ <Resource name="planAfter">
+ <![CDATA[
+LogicalProject(EXPR$0=[+($1, $4)])
+ LogicalJoin(condition=[AND(IS NOT DISTINCT FROM($0, $3), $2)],
joinType=[inner])
+ LogicalProject(ENAME=[$1], SAL=[$5], ==[=($7, 10)])
+ LogicalTableScan(table=[[CATALOG, SALES, EMP]])
+ LogicalProject(ENAME=[$0], COMM=[$3])
+ LogicalTableScan(table=[[CATALOG, SALES, BONUS]])
+]]>
+ </Resource>
+ </TestCase>
</Root>
