fix: map Calcite REAL to Substrait FP32 (#261)

BREAKING CHANGE: Substrait FP32 is now mapped to Calcite REAL instead of FLOAT BREAKING CHANGE: Calcite FLOAT is now mapped to Substrait FP64 instead of FP32 In Calcite, the Sql Type Names DOUBLE and FLOAT correspond to FP64, and REAL corresponds to FP32
substrait-io · May 31, 2024 · 37331c2 · 37331c2
1 parent 5389a86
commit 37331c2
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 10 deletions.
diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java
@@ -71,8 +71,8 @@ private Type toSubstrait(RelDataType type, List<String> names) {
       case SMALLINT -> creator.I16;
       case INTEGER -> creator.I32;
       case BIGINT -> creator.I64;
-      case FLOAT -> creator.FP32;
-      case DOUBLE -> creator.FP64;
+      case REAL -> creator.FP32;
+      case FLOAT, DOUBLE -> creator.FP64;
       case DECIMAL -> {
         if (type.getPrecision() > 38) {
           throw new UnsupportedOperationException(
@@ -203,7 +203,7 @@ public RelDataType visit(Type.I64 expr) {
 
     @Override
     public RelDataType visit(Type.FP32 expr) {
-      return t(n(expr), SqlTypeName.FLOAT);
+      return t(n(expr), SqlTypeName.REAL);
     }
 
     @Override

diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java
@@ -104,8 +104,8 @@ public Expression.Literal convert(RexLiteral literal) {
         }
         throw new UnsupportedOperationException("Unable to handle char type: " + val);
       }
-      case DOUBLE -> fp64(n, bd(literal).doubleValue());
-      case FLOAT -> fp32(n, bd(literal).floatValue());
+      case FLOAT, DOUBLE -> fp64(n, bd(literal).doubleValue());
+      case REAL -> fp32(n, bd(literal).floatValue());
 
       case DECIMAL -> {
         BigDecimal bd = bd(literal);

diff --git a/isthmus/src/test/java/io/substrait/isthmus/ArithmeticFunctionTest.java b/isthmus/src/test/java/io/substrait/isthmus/ArithmeticFunctionTest.java
@@ -8,7 +8,7 @@ public class ArithmeticFunctionTest extends PlanTestBase {
 
   static List<String> CREATES =
       List.of(
-          "CREATE TABLE numbers (i8 TINYINT, i16 SMALLINT, i32 INT, i64 BIGINT, fp32 FLOAT, fp64 DOUBLE)");
+          "CREATE TABLE numbers (i8 TINYINT, i16 SMALLINT, i32 INT, i64 BIGINT, fp32 REAL, fp64 DOUBLE)");
 
   @ParameterizedTest
   @ValueSource(strings = {"i8", "i16", "i32", "i64", "fp32", "fp64"})

diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java
@@ -70,14 +70,19 @@ void tI64() {
 
   @Test
   void tFP32() {
-    bitest(fp32(false, 4.44F), c(4.44F, SqlTypeName.FLOAT));
+    bitest(fp32(false, 4.44F), c(4.44F, SqlTypeName.REAL));
   }
 
   @Test
   void tFP64() {
     bitest(fp64(false, 4.45F), c(4.45F, SqlTypeName.DOUBLE));
   }
 
+  @Test
+  void tFloatFP64() {
+    test(fp64(false, 4.45F), c(4.45F, SqlTypeName.FLOAT));
+  }
+
   @Test
   void tStr() {
     bitest(string(false, "my test"), c("my test", SqlTypeName.VARCHAR));

diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java
@@ -51,7 +51,7 @@ void i64(boolean nullable) {
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void fp32(boolean nullable) {
-    testType(Type.withNullability(nullable).FP32, SqlTypeName.FLOAT, nullable);
+    testType(Type.withNullability(nullable).FP32, SqlTypeName.REAL, nullable);
   }
 
   @ParameterizedTest
@@ -60,6 +60,15 @@ void fp64(boolean nullable) {
     testType(Type.withNullability(nullable).FP64, SqlTypeName.DOUBLE, nullable);
   }
 
+  @ParameterizedTest
+  @ValueSource(booleans = {true, false})
+  void calciteFloatToFp64(boolean nullable) {
+    assertEquals(
+        Type.withNullability(nullable).FP64,
+        TypeConverter.DEFAULT.toSubstrait(
+            type.createTypeWithNullability(type.createSqlType(SqlTypeName.FLOAT), nullable)));
+  }
+
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void date(boolean nullable) {

diff --git a/isthmus/src/test/java/io/substrait/isthmus/LogarithmicFunctionTest.java b/isthmus/src/test/java/io/substrait/isthmus/LogarithmicFunctionTest.java
@@ -8,7 +8,7 @@ public class LogarithmicFunctionTest extends PlanTestBase {
 
   static List<String> CREATES =
       List.of(
-          "CREATE TABLE numbers (i8 TINYINT, i16 SMALLINT, i32 INT, i64 BIGINT, fp32 FLOAT, fp64 DOUBLE)");
+          "CREATE TABLE numbers (i8 TINYINT, i16 SMALLINT, i32 INT, i64 BIGINT, fp32 REAL, fp64 DOUBLE)");
 
   @ParameterizedTest
   @ValueSource(strings = {"fp32", "fp64"})

diff --git a/isthmus/src/test/java/io/substrait/isthmus/RoundingFunctionTest.java b/isthmus/src/test/java/io/substrait/isthmus/RoundingFunctionTest.java
@@ -8,7 +8,7 @@ public class RoundingFunctionTest extends PlanTestBase {
 
   static List<String> CREATES =
       List.of(
-          "CREATE TABLE numbers (i8 TINYINT, i16 SMALLINT, i32 INT, i64 BIGINT, fp32 FLOAT, fp64 DOUBLE)");
+          "CREATE TABLE numbers (i8 TINYINT, i16 SMALLINT, i32 INT, i64 BIGINT, fp32 REAL, fp64 DOUBLE)");
 
   @ParameterizedTest
   @ValueSource(strings = {"fp32", "fp64"})