[CALCITE-6265] Type coercion is failing for numeric values in prepare…

…d statements Address some issues on the original patch
rubenada · Apr 16, 2024 · 680694f · 680694f
1 parent e7c1f0c
commit 680694f
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 33 deletions.
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumUtils.java
@@ -354,6 +354,9 @@ public static Expression convert(Expression operand, Type fromType,
     if (!Types.needTypeCast(fromType, toType)) {
       return operand;
     }
+
+    // TODO use Expressions#convertChecked to throw exception in case of overflow (CALCITE-6366)
+
     // E.g. from "Short" to "int".
     // Generate "x.intValue()".
     final Primitive toPrimitive = Primitive.of(toType);

diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
@@ -31,7 +31,6 @@
 import org.apache.calcite.linq4j.tree.ParameterExpression;
 import org.apache.calcite.linq4j.tree.Primitive;
 import org.apache.calcite.linq4j.tree.Statement;
-import org.apache.calcite.linq4j.tree.Types;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rex.RexBuilder;
 import org.apache.calcite.rex.RexCall;
@@ -1393,20 +1392,17 @@ private Result toInnerStorageType(Result result, Type storageType) {
 
     // For numeric types, use java.lang.Number to prevent cast exception
     // when the parameter type differs from the target type
-    Expression argumentExpression =
-        EnumUtils.convert(
+    final Expression valueExpression = isNumeric
+        ? EnumUtils.convert(
+            EnumUtils.convert(
+                Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
+                    Expressions.constant("?" + dynamicParam.getIndex())),
+                java.lang.Number.class),
+            storageType)
+        : EnumUtils.convert(
             Expressions.call(root, BuiltInMethod.DATA_CONTEXT_GET.method,
                 Expressions.constant("?" + dynamicParam.getIndex())),
-            isNumeric ? java.lang.Number.class : storageType);
-
-    // Short-circuit if the expression evaluates to null. The cast
-    // may throw a NullPointerException as it calls methods on the
-    // object such as longValue().
-    Expression valueExpression =
-        Expressions.condition(
-            Expressions.equal(argumentExpression, Expressions.constant(null)),
-            Expressions.constant(null),
-            Types.castIfNecessary(storageType, argumentExpression));
+            storageType);
 
     final ParameterExpression valueVariable =
         Expressions.parameter(valueExpression.getType(),

diff --git a/core/src/test/java/org/apache/calcite/test/JdbcTest.java b/core/src/test/java/org/apache/calcite/test/JdbcTest.java
@@ -8453,6 +8453,33 @@ private void checkGetTimestamp(Connection con) throws SQLException {
     }
   }
 
+  @Test void bindDecimalParameter() {
+    final String sql =
+        "with cte as (select 2500.55 as val)"
+            + "select * from cte where val = ?";
+
+    CalciteAssert.hr()
+        .query(sql)
+        .consumesPreparedStatement(p -> {
+          p.setBigDecimal(1, new BigDecimal("2500.55"));
+        })
+        .returnsUnordered("VAL=2500.55");
+  }
+
+  @Test void bindNullParameter() {
+    final String sql =
+        "with cte as (select 2500.55 as val)"
+            + "select * from cte where val = ?";
+
+    CalciteAssert.hr()
+        .query(sql)
+        .consumesPreparedStatement(p -> {
+          p.setBigDecimal(1, null);
+        })
+        .returnsUnordered("");
+  }
+
+  @Disabled("CALCITE-6366")
   @Test void bindOverflowingTinyIntParameter() {
     final String sql =
         "with cte as (select cast(300 as smallint) as empid)"

diff --git a/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Types.java b/linq4j/src/main/java/org/apache/calcite/linq4j/tree/Types.java
@@ -28,7 +28,6 @@
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.TypeVariable;
-import java.math.BigDecimal;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -429,25 +428,11 @@ public static Expression castIfNecessary(Type returnType,
         && Number.class.isAssignableFrom((Class) returnType)
         && type instanceof Class
         && Number.class.isAssignableFrom((Class) type)) {
-
-      if (returnType == BigDecimal.class) {
-        return Expressions.call(
-            BigDecimal.class,
-            "valueOf",
-            Expressions.call(expression, "longValue"));
-      } else if (
-          returnType == Byte.class
-              || returnType == Short.class
-              || returnType == Integer.class
-              || returnType == Long.class) {
-        return Expressions.convertChecked(expression, returnType);
-      } else {
-        // E.g.
-        //   Integer foo(BigDecimal o) {
-        //     return o.intValue();
-        //   }
-        return Expressions.unbox(expression, requireNonNull(Primitive.ofBox(returnType)));
-      }
+      // E.g.
+      //   Integer foo(BigDecimal o) {
+      //     return o.intValue();
+      //   }
+      return Expressions.unbox(expression, requireNonNull(Primitive.ofBox(returnType)));
     }
     if (Primitive.is(returnType) && !Primitive.is(type)) {
       // E.g.