[CALCITE-739] Extend RexUtil.pullFactors to recognize additional common factors

core/src/main/java/org/apache/calcite/rex/RexSimplify.java

@@ -657,6 +657,52 @@ private String simplifyLikeString(String content, char escape, char wildcard) {
     return simplifyMixedWildcards(builder.toString(), escape);
   }
 
+  /** Returns whether two nodes are the same comparison, considering the
+   * symmetry of comparison operators: "{@code a = b}" is equivalent to
+   * "{@code b = a}", and "{@code a < b}" is equivalent to
+   * "{@code b > a}".
+   *
+   * <p>This allows digest-based rewrites such as the absorption law
+   * ("{@code a AND (a OR b) => a}") to recognize comparison terms that
+   * differ only in the order of their operands.
+   * See <a href="https://issues.apache.org/jira/browse/CALCITE-739">[CALCITE-739]
+   * Extend RexUtil.pullFactors to recognize additional common factors</a>.
+   */
+  private static boolean equivalentComparison(RexNode a, RexNode b) {
+    if (a.equals(b)) {
+      return true;
+    }
+    if (!(a instanceof RexCall) || !(b instanceof RexCall)) {
+      return false;
+    }
+    final RexCall callA = (RexCall) a;
+    final RexCall callB = (RexCall) b;
+    final SqlKind kindA = callA.getKind();
+    // Comparison operators are the only operators whose semantics is
+    // preserved when operands are reversed and the operator is reversed
+    // (for example, "a < b" becomes "b > a").
+    if (!SqlKind.COMPARISON.contains(kindA)
+        || kindA.reverse() != callB.getKind()
+        || callA.getOperands().size() != 2
+        || callB.getOperands().size() != 2) {
+      return false;
+    }
+    return callA.getOperands().get(0).equals(callB.getOperands().get(1))
+        && callA.getOperands().get(1).equals(callB.getOperands().get(0));
+  }
+
+  /** Returns whether {@code nodes} contains a node that is the same
+   * comparison as {@code target}; see {@link #equivalentComparison}. */
+  private static boolean containsEquivalentComparison(List<RexNode> nodes,
+      RexNode target) {
+    for (RexNode node : nodes) {
+      if (equivalentComparison(node, target)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   // e must be a comparison (=, >, >=, <, <=, !=)
   private RexNode simplifyComparison(RexCall e, RexUnknownAs unknownAs) {
     //noinspection unchecked
@@ -2461,8 +2507,8 @@ private static void absorb(List<RexNode> terms, SqlKind compositeKind) {
             ? RelOptUtil.disjunctions(term)
             : RelOptUtil.conjunctions(term);
         for (RexNode other : terms) {
-          if (other != term && components.contains(other)
-              && RexUtil.isDeterministic(other)) {
+          if (other != term && RexUtil.isDeterministic(other)
+              && containsEquivalentComparison(components, other)) {
             terms.remove(i);
             i--;
             break;

core/src/main/java/org/apache/calcite/rex/RexUtil.java

@@ -2967,7 +2967,7 @@ private RexNode pull(RexNode rex) {
         }
         return and(Iterables.concat(factors.values(), ImmutableList.of(or(list))));
       default:
-        return rex;
+        return normalizeComparison(rex);
       }
     }
 
@@ -2986,17 +2986,19 @@ private List<RexNode> pullList(List<RexNode> nodes) {
       return list;
     }
 
-    private static LinkedHashMap<RexNode, RexNode> commonFactors(List<RexNode> nodes) {
+    private LinkedHashMap<RexNode, RexNode> commonFactors(List<RexNode> nodes) {
       // make sure the result is in deterministic order
       final LinkedHashMap<RexNode, RexNode> map = new LinkedHashMap<>();
       int i = 0;
       for (RexNode node : nodes) {
         if (i++ == 0) {
           for (RexNode conjunction : RelOptUtil.conjunctions(node)) {
-            map.put(conjunction, conjunction);
+            RexNode normalized = normalizeComparison(conjunction);
+            map.put(normalized, normalized);
           }
         } else {
-          map.keySet().retainAll(RelOptUtil.conjunctions(node));
+          map.keySet().retainAll(
+              Util.transform(RelOptUtil.conjunctions(node), this::normalizeComparison));
         }
       }
       return map;
@@ -3005,13 +3007,58 @@ private static LinkedHashMap<RexNode, RexNode> commonFactors(List<RexNode> nodes
     private RexNode removeFactor(Map<RexNode, RexNode> factors, RexNode node) {
       List<RexNode> list = new ArrayList<>();
       for (RexNode operand : RelOptUtil.conjunctions(node)) {
-        if (!factors.containsKey(operand)) {
+        RexNode normalized = normalizeComparison(operand);
+        if (!factors.containsKey(normalized)) {
           list.add(operand);
         }
       }
       return and(list);
     }
 
+    /**
+     * Normalizes a comparison expression so that, when possible, an input ref
+     * appears on the left and a literal or higher-index input ref appears on
+     * the right. This exploits the symmetry of comparisons to help
+     * {@link #commonFactors} recognize equivalent terms.
+     *
+     * <p>Only comparisons that involve an input ref are normalized: if both
+     * operands are input refs, the lower-index one is placed on the left;
+     * if exactly one operand is an input ref, it is placed on the left.
+     * Comparisons whose operands are not input refs (for example, two
+     * CASTs) are returned unchanged.
+     */
+    private RexNode normalizeComparison(RexNode rex) {
+      if (rex instanceof RexCall) {
+        RexCall call = (RexCall) rex;
+        switch (call.getKind()) {
+        case EQUALS:
+        case NOT_EQUALS:
+        case LESS_THAN:
+        case GREATER_THAN:
+        case LESS_THAN_OR_EQUAL:
+        case GREATER_THAN_OR_EQUAL:
+          final List<RexNode> operands = call.getOperands();
+          final RexNode op0 = operands.get(0);
+          final RexNode op1 = operands.get(1);
+          final boolean op0IsInputRef = op0 instanceof RexInputRef;
+          final boolean op1IsInputRef = op1 instanceof RexInputRef;
+          if (op0IsInputRef && op1IsInputRef) {
+            final RexInputRef ref0 = (RexInputRef) op0;
+            final RexInputRef ref1 = (RexInputRef) op1;
+            if (ref0.getIndex() > ref1.getIndex()) {
+              return requireNonNull(invert(rexBuilder, call));
+            }
+          } else if (!op0IsInputRef && op1IsInputRef) {
+            return requireNonNull(invert(rexBuilder, call));
+          }
+          break;
+        default:
+          break;
+        }
+      }
+      return rex;
+    }
+
     private RexNode and(Iterable<? extends RexNode> nodes) {
       return composeConjunction(rexBuilder, nodes);
     }

core/src/test/java/org/apache/calcite/rex/RexProgramTest.java

@@ -889,6 +889,19 @@ private void checkExponentialCnf(int n) {
             + " 'MED PKG':CHAR(8)]:CHAR(8))), AND(=(?0.j, 'Brand#14'), "
             + ">=(?0.h, 20), <=(?0.h, 30), SEARCH(?0.k, Sarg['LG BOX':CHAR(7),"
             + " 'LG CASE', 'LG PACK', 'LG PKG':CHAR(7)]:CHAR(7)))))");
+
+    final RexNode ref3 = rexBuilder.makeInputRef(intType, 3);
+    final RexNode ref6 = rexBuilder.makeInputRef(intType, 6);
+    final RexNode bool0 = rexBuilder.makeInputRef(booleanType, 0);
+    final RexNode bool1 = rexBuilder.makeInputRef(booleanType, 1);
+    final RexNode seven = literal(7);
+    checkPullFactors(eq(ref6, ref3), "=($3, $6)");
+    checkPullFactors(gt(ref6, ref3), "<($3, $6)");
+    checkPullFactors(le(seven, ref3), ">=($3, 7)");
+    checkPullFactors(
+        or(and(eq(ref6, ref3), bool0),
+            and(eq(ref3, ref6), bool1)),
+        "AND(=($3, $6), OR($0, $1))");
   }
 
   @Test void testSimplify() {
@@ -2342,6 +2355,28 @@ private void checkExponentialCnf(int n) {
     }
   }
 
+  /** Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-739">[CALCITE-739]
+   * Extend RexUtil.pullFactors to recognize additional common factors</a>. */
+  @Test void testSimplifyComparisonSymmetry() {
+    final RexNode ref3 = rexBuilder.makeInputRef(tInt(), 3);
+    final RexNode ref6 = rexBuilder.makeInputRef(tInt(), 6);
+    final RexNode bool0 = rexBuilder.makeInputRef(tBool(), 0);
+
+    // Contradiction detection: "$6 = $3 AND $3 != $6" is not satisfiable.
+    checkSimplifyFilter(and(eq(ref6, ref3), ne(ref3, ref6)), "false");
+
+    // Absorption law: "a AND (a OR b) => a" where "a" is written once as
+    // "$6 = $3" and once as "$3 = $6". The result keeps the first form.
+    checkSimplify(and(eq(ref6, ref3), or(eq(ref3, ref6), bool0)),
+        "=($6, $3)");
+
+    // Same, but with an order-sensitive operator: "a" is "$6 > $3",
+    // written inside the OR as "$3 < $6".
+    checkSimplify(and(gt(ref6, ref3), or(lt(ref3, ref6), bool0)),
+        ">($6, $3)");
+  }
+
   @Test void testSimplifyComparisonWithPredicates() {
     RelOptPredicateList relOptPredicateList =
         RelOptPredicateList.of(rexBuilder,