You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2018/12/03 21:20:39 UTC

[3/6] calcite git commit: [CALCITE-2688] Improve diagnosability when validator cannot infer a return type (Zoltan Haindrich)

[CALCITE-2688] Improve diagnosability when validator cannot infer a return type (Zoltan Haindrich)

Close apache/calcite#930


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

Branch: refs/heads/master
Commit: e274f1ba28494a037fd6e06d7a7b8d762661484f
Parents: 5ebf458
Author: Zoltan Haindrich <ki...@rxd.hu>
Authored: Tue Nov 20 09:28:20 2018 +0100
Committer: Julian Hyde <jh...@apache.org>
Committed: Mon Dec 3 10:56:02 2018 -0800

----------------------------------------------------------------------
 .../main/java/org/apache/calcite/sql/SqlOperator.java   |  8 +++++++-
 .../main/java/org/apache/calcite/tools/RelBuilder.java  |  5 -----
 .../java/org/apache/calcite/test/RelBuilderTest.java    |  8 ++++----
 .../java/org/apache/calcite/test/RelMetadataTest.java   |  1 -
 .../java/org/apache/calcite/test/RexProgramTest.java    | 12 ++++++++++++
 5 files changed, 23 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/e274f1ba/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
index c4c092e..eeee2c3 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlOperator.java
@@ -467,7 +467,13 @@ public abstract class SqlOperator {
   public RelDataType inferReturnType(
       SqlOperatorBinding opBinding) {
     if (returnTypeInference != null) {
-      return returnTypeInference.inferReturnType(opBinding);
+      RelDataType returnType = returnTypeInference.inferReturnType(opBinding);
+      if (returnType == null) {
+        throw new IllegalArgumentException("Cannot infer return type for "
+            + opBinding.getOperator() + "; operand types: "
+            + opBinding.collectOperandTypes());
+      }
+      return returnType;
     }
 
     // Derived type should have overridden this method, since it didn't

http://git-wip-us.apache.org/repos/asf/calcite/blob/e274f1ba/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
index 9b7a6e9..0465286 100644
--- a/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
+++ b/core/src/main/java/org/apache/calcite/tools/RelBuilder.java
@@ -556,11 +556,6 @@ public class RelBuilder {
   private RexNode call(SqlOperator operator, List<RexNode> operandList) {
     final RexBuilder builder = cluster.getRexBuilder();
     final RelDataType type = builder.deriveReturnType(operator, operandList);
-    if (type == null) {
-      throw new IllegalArgumentException("cannot derive type: " + operator
-          + "; operands: "
-          + Lists.transform(operandList, e -> e + ": " + e.getType()));
-    }
     return builder.makeCall(type, operator, operandList);
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/e274f1ba/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
index ff3f861..77a91f7 100644
--- a/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelBuilderTest.java
@@ -401,8 +401,8 @@ public class RelBuilderTest {
       fail("expected error, got " + call);
     } catch (IllegalArgumentException e) {
       assertThat(e.getMessage(),
-          is("cannot derive type: +; "
-              + "operands: [$1: VARCHAR(10), $3: SMALLINT]"));
+          is("Cannot infer return type for +; "
+              + "operand types: [VARCHAR(10), SMALLINT]"));
     }
   }
 
@@ -2057,7 +2057,7 @@ public class RelBuilderTest {
       builder.call(SqlStdOperatorTable.PLUS, Lists.newArrayList(arg0, arg1));
       fail("Invalid combination of parameter types");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage(), containsString("cannot derive type"));
+      assertThat(e.getMessage(), containsString("Cannot infer return type"));
     }
 
     // test for b) call(operator, RexNode...)
@@ -2065,7 +2065,7 @@ public class RelBuilderTest {
       builder.call(SqlStdOperatorTable.PLUS, arg0, arg1);
       fail("Invalid combination of parameter types");
     } catch (IllegalArgumentException e) {
-      assertThat(e.getMessage(), containsString("cannot derive type"));
+      assertThat(e.getMessage(), containsString("Cannot infer return type"));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/calcite/blob/e274f1ba/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
index 4628875..0706782 100644
--- a/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RelMetadataTest.java
@@ -87,7 +87,6 @@ import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.tools.FrameworkConfig;
 import org.apache.calcite.tools.Frameworks;
 import org.apache.calcite.tools.RelBuilder;
-import org.apache.calcite.tools.RelBuilder.AggCall;
 import org.apache.calcite.util.BuiltInMethod;
 import org.apache.calcite.util.ImmutableBitSet;
 import org.apache.calcite.util.ImmutableIntList;

http://git-wip-us.apache.org/repos/asf/calcite/blob/e274f1ba/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
index b52937f..0c5f549 100644
--- a/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
+++ b/core/src/test/java/org/apache/calcite/test/RexProgramTest.java
@@ -74,6 +74,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 /**
  * Unit tests for {@link RexProgram} and
@@ -748,6 +749,17 @@ public class RexProgramTest extends RexProgramBuilderBase {
     checkSimplifyUnchanged(cast(cast(vVarchar(), tInt()), tVarchar()));
   }
 
+  @Test public void testNoCommonReturnTypeFails() {
+    try {
+      final RexNode node = coalesce(vVarchar(1), vInt(2));
+      fail("expected exception, got " + node);
+    } catch (IllegalArgumentException e) {
+      final String expected = "Cannot infer return type for COALESCE;"
+          + " operand types: [VARCHAR, INTEGER]";
+      assertThat(e.getMessage(), is(expected));
+    }
+  }
+
   /** Unit test for {@link org.apache.calcite.rex.RexUtil#toCnf}. */
   @Test public void testCnf() {
     final RelDataType booleanType =