Repository: calcite
Updated Branches:
  refs/heads/master a1fe76911 -> 89f25e221


[CALCITE-2190] Extend SubstitutionVisitor.splitFilter to cover different order 
of operands

Close apache/calcite#635


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/89f25e22
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/89f25e22
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/89f25e22

Branch: refs/heads/master
Commit: 89f25e221de749766b0c49fb01ee589a595127ef
Parents: a1fe769
Author: Jesus Camacho Rodriguez <jcama...@apache.org>
Authored: Thu Feb 22 18:34:23 2018 -0800
Committer: Jesus Camacho Rodriguez <jcama...@apache.org>
Committed: Fri Feb 23 15:22:34 2018 -0800

----------------------------------------------------------------------
 .../calcite/plan/SubstitutionVisitor.java       | 56 +++++++++++++++++---
 .../calcite/test/MaterializationTest.java       | 43 ++++++++-------
 2 files changed, 74 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/89f25e22/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
----------------------------------------------------------------------
diff --git 
a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java 
b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
index d18527b..bdfe271 100644
--- a/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
+++ b/core/src/main/java/org/apache/calcite/plan/SubstitutionVisitor.java
@@ -79,6 +79,8 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
 
 import static org.apache.calcite.rex.RexUtil.andNot;
 import static org.apache.calcite.rex.RexUtil.removeAll;
@@ -272,28 +274,32 @@ public class SubstitutionVisitor {
   @VisibleForTesting
   public static RexNode splitFilter(final RexSimplify simplify,
       RexNode condition, RexNode target) {
+    RexNode condition2 = canonizeNode(simplify.rexBuilder, condition);
+    RexNode target2 = canonizeNode(simplify.rexBuilder, target);
+
     // First, try splitting into ORs.
     // Given target    c1 OR c2 OR c3 OR c4
     // and condition   c2 OR c4
     // residue is      c2 OR c4
     // Also deals with case target [x] condition [x] yields residue [true].
-    RexNode z = splitOr(simplify.rexBuilder, condition, target);
+    RexNode z = splitOr(simplify.rexBuilder, condition2, target2);
     if (z != null) {
       return z;
     }
 
-    if (isEquivalent(simplify.rexBuilder, condition, target)) {
+    if (isEquivalent(simplify.rexBuilder, condition2, target2)) {
       return simplify.rexBuilder.makeLiteral(true);
     }
 
-    RexNode x = andNot(simplify.rexBuilder, target, condition);
+    RexNode x = andNot(simplify.rexBuilder, target2, condition2);
     if (mayBeSatisfiable(x)) {
       RexNode x2 = RexUtil.composeConjunction(simplify.rexBuilder,
-          ImmutableList.of(condition, target), false);
-      RexNode r = simplify.withUnknownAsFalse(true).simplify(x2);
-      if (!r.isAlwaysFalse() && isEquivalent(simplify.rexBuilder, condition, 
r)) {
+          ImmutableList.of(condition2, target2), false);
+      RexNode r = canonizeNode(simplify.rexBuilder,
+          simplify.withUnknownAsFalse(true).simplify(x2));
+      if (!r.isAlwaysFalse() && isEquivalent(simplify.rexBuilder, condition2, 
r)) {
         List<RexNode> conjs = RelOptUtil.conjunctions(r);
-        for (RexNode e : RelOptUtil.conjunctions(target)) {
+        for (RexNode e : RelOptUtil.conjunctions(target2)) {
           removeAll(conjs, e);
         }
         return RexUtil.composeConjunction(simplify.rexBuilder, conjs, false);
@@ -302,6 +308,42 @@ public class SubstitutionVisitor {
     return null;
   }
 
+  /**
+   * Reorders some of the operands in this expression so structural comparison,
+   * i.e., based on string representation, can be more precise.
+   */
+  private static RexNode canonizeNode(RexBuilder rexBuilder, RexNode 
condition) {
+    switch (condition.getKind()) {
+    case AND:
+    case OR: {
+      RexCall call = (RexCall) condition;
+      SortedMap<String, RexNode> newOperands = new TreeMap<>();
+      for (RexNode operand : call.operands) {
+        operand = canonizeNode(rexBuilder, operand);
+        newOperands.put(operand.toString(), operand);
+      }
+      return rexBuilder.makeCall(call.getOperator(),
+          ImmutableList.copyOf(newOperands.values()));
+    }
+    case EQUALS:
+    case NOT_EQUALS:
+    case LESS_THAN:
+    case GREATER_THAN:
+    case LESS_THAN_OR_EQUAL:
+    case GREATER_THAN_OR_EQUAL: {
+      RexCall call = (RexCall) condition;
+      final RexNode left = call.getOperands().get(0);
+      final RexNode right = call.getOperands().get(1);
+      if (left.toString().compareTo(right.toString()) <= 0) {
+        return call;
+      }
+      return RexUtil.invert(rexBuilder, call);
+    }
+    default:
+      return condition;
+    }
+  }
+
   private static RexNode splitOr(
       final RexBuilder rexBuilder, RexNode condition, RexNode target) {
     List<RexNode> conditions = RelOptUtil.disjunctions(condition);

http://git-wip-us.apache.org/repos/asf/calcite/blob/89f25e22/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
----------------------------------------------------------------------
diff --git 
a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java 
b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
index 6c52f93..4ce0dc2 100644
--- a/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
+++ b/core/src/test/java/org/apache/calcite/test/MaterializationTest.java
@@ -782,7 +782,7 @@ public class MaterializationTest {
     final RexNode x_eq_1 =
         rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, x, i1); // $0 = 1
     final RexNode x_eq_1_b =
-        rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, x, i1); // $0 = 1 again
+        rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, i1, x); // 1 = $0
     final RexNode x_eq_2 =
         rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, x, i2); // $0 = 2
     final RexNode y_eq_2 =
@@ -793,7 +793,14 @@ public class MaterializationTest {
     RexNode newFilter;
 
     // Example 1.
-    // TODO:
+    //   condition: x = 1 or y = 2
+    //   target:    y = 2 or 1 = x
+    // yields
+    //   residue:   true
+    newFilter = SubstitutionVisitor.splitFilter(simplify,
+        rexBuilder.makeCall(SqlStdOperatorTable.OR, x_eq_1, y_eq_2),
+        rexBuilder.makeCall(SqlStdOperatorTable.OR, y_eq_2, x_eq_1_b));
+    assertThat(newFilter.isAlwaysTrue(), equalTo(true));
 
     // Example 2.
     //   condition: x = 1,
@@ -1059,7 +1066,7 @@ public class MaterializationTest {
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
             "EnumerableAggregate(group=[{1}])\n"
-                + "  EnumerableCalc(expr#0..1=[{inputs}], expr#2=[10], 
expr#3=[>($t1, $t2)], "
+                + "  EnumerableCalc(expr#0..1=[{inputs}], expr#2=[10], 
expr#3=[<($t2, $t1)], "
                 + "proj#0..1=[{exprs}], $condition=[$t3])\n"
                 + "    EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1132,7 +1139,7 @@ public class MaterializationTest {
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
             "EnumerableAggregate(group=[{1}], S=[$SUM0($3)])\n"
-                + "  EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[>($t1, $t4)], "
+                + "  EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[<($t4, $t1)], "
                 + "proj#0..3=[{exprs}], $condition=[$t5])\n"
                 + "    EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1148,7 +1155,7 @@ public class MaterializationTest {
             "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[1], expr#3=[+($t1, 
$t2)],"
                 + " deptno=[$t0], $f1=[$t3])\n"
                 + "  EnumerableAggregate(group=[{1}], agg#0=[$SUM0($3)])\n"
-                + "    EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[>($t1, $t4)], "
+                + "    EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[<($t4, $t1)], "
                 + "proj#0..3=[{exprs}], $condition=[$t5])\n"
                 + "      EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1173,8 +1180,8 @@ public class MaterializationTest {
             "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[1], expr#3=[+($t0, 
$t2)], "
                 + "expr#4=[+($t1, $t2)], $f0=[$t3], $f1=[$t4])\n"
                 + "  EnumerableAggregate(group=[{1}], agg#0=[$SUM0($3)])\n"
-                + "    EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], "
-                + "expr#5=[>($t1, $t4)], proj#0..3=[{exprs}], 
$condition=[$t5])\n"
+                + "    EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[<($t4, $t1)], "
+                + "proj#0..3=[{exprs}], $condition=[$t5])\n"
                 + "      EnumerableTableScan(table=[[hr, m0]])"));
   }
 
@@ -1290,7 +1297,7 @@ public class MaterializationTest {
             + "group by \"empid\", \"depts\".\"deptno\"",
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
-            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[20], expr#3=[>($t1, 
$t2)], "
+            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[20], expr#3=[<($t2, 
$t1)], "
                 + "empid=[$t0], $condition=[$t3])\n"
                 + "  EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1305,7 +1312,7 @@ public class MaterializationTest {
             + "group by \"empid\", \"depts\".\"deptno\"",
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
-            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[20], expr#3=[>($t0, 
$t2)], "
+            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[20], expr#3=[<($t2, 
$t0)], "
                 + "empid=[$t1], $condition=[$t3])\n"
                 + "  EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1332,7 +1339,7 @@ public class MaterializationTest {
             + "group by \"empid\", \"depts\".\"deptno\"",
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
-            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[20], expr#3=[>($t1, 
$t2)], "
+            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[20], expr#3=[<($t2, 
$t1)], "
                 + "empid=[$t0], $condition=[$t3])\n"
                 + "  EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1347,7 +1354,7 @@ public class MaterializationTest {
             + "group by \"depts\".\"deptno\", \"emps\".\"empid\"",
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
-            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[15], expr#3=[>($t1, 
$t2)], "
+            "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[15], expr#3=[<($t2, 
$t1)], "
                 + "deptno=[$t0], $condition=[$t3])\n"
                 + "  EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1363,7 +1370,7 @@ public class MaterializationTest {
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
             "EnumerableAggregate(group=[{0}])\n"
-                + "  EnumerableCalc(expr#0..1=[{inputs}], expr#2=[15], 
expr#3=[>($t1, $t2)], "
+                + "  EnumerableCalc(expr#0..1=[{inputs}], expr#2=[15], 
expr#3=[<($t2, $t1)], "
                 + "proj#0..1=[{exprs}], $condition=[$t3])\n"
                 + "    EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1390,7 +1397,7 @@ public class MaterializationTest {
             "EnumerableUnion(all=[true])",
             "EnumerableAggregate(group=[{2}])",
             "EnumerableTableScan(table=[[hr, m0]])",
-            "expr#5=[10], expr#6=[>($t0, $t5)], expr#7=[11], expr#8=[<=($t0, 
$t7)]"));
+            "expr#5=[10], expr#6=[>($t0, $t5)], expr#7=[11], expr#8=[>=($t7, 
$t0)]"));
   }
 
   @Test public void testJoinAggregateMaterializationNoAggregateFuncs8() {
@@ -1457,8 +1464,8 @@ public class MaterializationTest {
         CalciteAssert.checkResultContains(
             "EnumerableAggregate(group=[{4}])\n"
                 + "  EnumerableCalc(expr#0..4=[{inputs}], expr#5=[=($t2, 
$t3)], "
-                + "expr#6=[CAST($t0):VARCHAR CHARACTER SET \"ISO-8859-1\" 
COLLATE \"ISO-8859-1$en_US$primary\"], "
-                + "expr#7=[CAST($t1):VARCHAR CHARACTER SET \"ISO-8859-1\" 
COLLATE \"ISO-8859-1$en_US$primary\"], "
+                + "expr#6=[CAST($t1):VARCHAR CHARACTER SET \"ISO-8859-1\" 
COLLATE \"ISO-8859-1$en_US$primary\"], "
+                + "expr#7=[CAST($t0):VARCHAR CHARACTER SET \"ISO-8859-1\" 
COLLATE \"ISO-8859-1$en_US$primary\"], "
                 + "expr#8=[=($t6, $t7)], expr#9=[AND($t5, $t8)], 
proj#0..4=[{exprs}], $condition=[$t9])\n"
                 + "    EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1516,7 +1523,7 @@ public class MaterializationTest {
         HR_FKUK_MODEL,
         CalciteAssert.checkResultContains(
             "EnumerableAggregate(group=[{1}], S=[$SUM0($3)])\n"
-                + "  EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[>($t1, $t4)], "
+                + "  EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[<($t4, $t1)], "
                 + "proj#0..3=[{exprs}], $condition=[$t5])\n"
                 + "    EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1534,7 +1541,7 @@ public class MaterializationTest {
             "EnumerableCalc(expr#0..1=[{inputs}], expr#2=[1], expr#3=[+($t1, 
$t2)], "
                 + "deptno=[$t0], S=[$t3])\n"
                 + "  EnumerableAggregate(group=[{1}], agg#0=[$SUM0($3)])\n"
-                + "    EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[>($t1, $t4)], "
+                + "    EnumerableCalc(expr#0..3=[{inputs}], expr#4=[10], 
expr#5=[<($t4, $t1)], "
                 + "proj#0..3=[{exprs}], $condition=[$t5])\n"
                 + "      EnumerableTableScan(table=[[hr, m0]])"));
   }
@@ -1800,7 +1807,7 @@ public class MaterializationTest {
         CalciteAssert.checkResultContains(
             "EnumerableUnion(all=[true])",
             "EnumerableTableScan(table=[[hr, m0]])",
-            "expr#5=[10], expr#6=[>($t0, $t5)], expr#7=[30], expr#8=[<=($t0, 
$t7)]"));
+            "expr#5=[10], expr#6=[>($t0, $t5)], expr#7=[30], expr#8=[>=($t7, 
$t0)]"));
   }
 
   @Test public void testJoinMaterializationUKFK1() {

Reply via email to