[calcite] branch master updated: [CALCITE-3809] RexSimplify simplifies nondeterministic function incorrectly

[chunwei]

This is an automated email from the ASF dual-hosted git repository.

chunwei 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 200a136  [CALCITE-3809] RexSimplify simplifies nondeterministic 
function incorrectly
200a136 is described below

commit 200a136dd3b2ca04a1d1283eb5a03a04388b1947
Author: Chunwei Lei <chunwei.l...@gmail.com>
AuthorDate: Thu Feb 20 22:56:29 2020 +0800

    [CALCITE-3809] RexSimplify simplifies nondeterministic function incorrectly
---
 .../main/java/org/apache/calcite/rex/RexAnalyzer.java  |  6 ++----
 .../java/org/apache/calcite/rex/RexInterpreter.java    | 12 ++++++++++++
 .../main/java/org/apache/calcite/rex/RexSimplify.java  |  2 +-
 .../java/org/apache/calcite/rex/RexProgramTest.java    | 18 ++++++++++++++++++
 4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java 
b/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java
index efbd723..dea076c 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexAnalyzer.java
@@ -128,13 +128,11 @@ public class RexAnalyzer {
     }
 
     @Override public Void visitCall(RexCall call) {
-      switch (call.getKind()) {
-      case CAST:
+      if (!RexInterpreter.SUPPORTED_SQL_KIND.contains(call.getKind())) {
         ++unsupportedCount;
         return null;
-      default:
-        return super.visitCall(call);
       }
+      return super.visitCall(call);
     }
   }
 }
diff --git a/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java 
b/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
index 2d1f761..e3c3240 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexInterpreter.java
@@ -20,6 +20,7 @@ import org.apache.calcite.avatica.util.DateTimeUtils;
 import org.apache.calcite.avatica.util.TimeUnit;
 import org.apache.calcite.avatica.util.TimeUnitRange;
 import org.apache.calcite.rel.metadata.NullSentinel;
+import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.util.NlsString;
 import org.apache.calcite.util.Util;
 
@@ -29,6 +30,7 @@ import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Comparator;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Map;
 import java.util.function.IntPredicate;
@@ -47,6 +49,16 @@ import java.util.function.IntPredicate;
 public class RexInterpreter implements RexVisitor<Comparable> {
   private static final NullSentinel N = NullSentinel.INSTANCE;
 
+  public static final EnumSet<SqlKind> SUPPORTED_SQL_KIND =
+      EnumSet.of(SqlKind.IS_NOT_DISTINCT_FROM, SqlKind.EQUALS, 
SqlKind.IS_DISTINCT_FROM,
+          SqlKind.NOT_EQUALS, SqlKind.GREATER_THAN, 
SqlKind.GREATER_THAN_OR_EQUAL,
+          SqlKind.LESS_THAN, SqlKind.LESS_THAN_OR_EQUAL, SqlKind.AND, 
SqlKind.OR,
+          SqlKind.NOT, SqlKind.CASE, SqlKind.IS_TRUE, SqlKind.IS_NOT_TRUE,
+          SqlKind.IS_FALSE, SqlKind.IS_NOT_FALSE, SqlKind.PLUS_PREFIX,
+          SqlKind.MINUS_PREFIX, SqlKind.PLUS, SqlKind.MINUS, SqlKind.TIMES,
+          SqlKind.DIVIDE, SqlKind.COALESCE, SqlKind.CEIL,
+          SqlKind.FLOOR, SqlKind.EXTRACT);
+
   private final Map<RexNode, Comparable> environment;
 
   /** Creates an interpreter.
diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java 
b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
index a8064df..becfe66 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java
@@ -323,7 +323,7 @@ public class RexSimplify {
     // Simplify "x <op> x"
     final RexNode o0 = operands.get(0);
     final RexNode o1 = operands.get(1);
-    if (o0.equals(o1)) {
+    if (o0.equals(o1) && RexUtil.isDeterministic(o0)) {
       RexNode newExpr;
       switch (e.getKind()) {
       case EQUALS:
diff --git a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java 
b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
index d9c9c75..76b82eb 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexProgramTest.java
@@ -2550,4 +2550,22 @@ public class RexProgramTest extends RexProgramTestBase {
         ImmutableList.of(gt(ref, literal(1)), le(ref, literal(5))));
     checkSimplifyFilter(gt(ref, literal(9)), relOptPredicateList, "false");
   }
+
+  @Test public void testSimplifyNonDeterministicFunction() {
+    final SqlOperator ndc = new SqlSpecialOperator(
+        "NDC",
+        SqlKind.OTHER_FUNCTION,
+        0,
+        false,
+        ReturnTypes.BOOLEAN,
+        null, null) {
+      @Override public boolean isDeterministic() {
+        return false;
+      }
+    };
+    final RexNode call1 = rexBuilder.makeCall(ndc);
+    final RexNode call2 = rexBuilder.makeCall(ndc);
+    final RexNode expr = eq(call1, call2);
+    checkSimplifyUnchanged(expr);
+  }
 }