Disable predicates pushdown to MemSQL REAL type

Previously, it returned incorrect result: CREAT TABLE test (c1 real); INSERT INTO test VALUES (real '3.14'); SELECT * FROM test; -- returns 3.14 SELECT * FROM test WHERE c1 = real '3.14'; -- no result
trinodb · Nov 19, 2021 · 4df037a · 4df037a
1 parent 44c3854
commit 4df037a
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 21 deletions.
diff --git a/plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java b/plugin/trino-memsql/src/main/java/io/trino/plugin/memsql/MemSqlClient.java
@@ -87,7 +87,6 @@
 import static io.trino.plugin.jdbc.StandardColumnMappings.defaultVarcharColumnMapping;
 import static io.trino.plugin.jdbc.StandardColumnMappings.doubleColumnMapping;
 import static io.trino.plugin.jdbc.StandardColumnMappings.integerColumnMapping;
-import static io.trino.plugin.jdbc.StandardColumnMappings.realColumnMapping;
 import static io.trino.plugin.jdbc.StandardColumnMappings.realWriteFunction;
 import static io.trino.plugin.jdbc.StandardColumnMappings.smallintColumnMapping;
 import static io.trino.plugin.jdbc.StandardColumnMappings.timeColumnMappingUsingSqlTime;
@@ -106,6 +105,7 @@
 import static io.trino.spi.type.TimestampType.TIMESTAMP_MICROS;
 import static io.trino.spi.type.TimestampType.createTimestampType;
 import static io.trino.spi.type.VarbinaryType.VARBINARY;
+import static java.lang.Float.floatToRawIntBits;
 import static java.lang.Math.max;
 import static java.lang.Math.min;
 import static java.lang.String.format;
@@ -305,7 +305,13 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect
             case Types.BIGINT:
                 return Optional.of(bigintColumnMapping());
             case Types.REAL:
-                return Optional.of(realColumnMapping());
+                // Disable pushdown because floating-point values are approximate and not stored as exact values,
+                // attempts to treat them as exact in comparisons may lead to problems
+                return Optional.of(ColumnMapping.longMapping(
+                        REAL,
+                        (resultSet, columnIndex) -> floatToRawIntBits(resultSet.getFloat(columnIndex)),
+                        realWriteFunction(),
+                        DISABLE_PUSHDOWN));
             case Types.DOUBLE:
                 return Optional.of(doubleColumnMapping());
             case Types.CHAR:

diff --git a/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlConnectorTest.java b/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlConnectorTest.java
@@ -134,8 +134,7 @@ protected Optional<DataMappingTestSetup> filterDataMappingSmokeTestData(DataMapp
             return Optional.of(dataMappingTestSetup.asUnsupported());
         }
 
-        if (typeName.equals("real")
-                || typeName.equals("timestamp")) {
+        if (typeName.equals("timestamp")) {
             // TODO this should either work or fail cleanly
             return Optional.empty();
         }

diff --git a/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlTypeMapping.java b/plugin/trino-memsql/src/test/java/io/trino/plugin/memsql/TestMemSqlTypeMapping.java
@@ -147,20 +147,20 @@ public void testBoolean()
     @Test
     public void testFloat()
     {
-        singlePrecisionFloatingPointTests(realDataType())
+        // we are not testing Nan/-Infinity/+Infinity as those are not supported by MemSQL
+        SqlDataTypeTest.create()
+                .addRoundTrip("real", "3.14", REAL, "REAL '3.14'")
+                .addRoundTrip("real", "10.3e0", REAL, "REAL '10.3e0'")
+                .addRoundTrip("real", "NULL", REAL, "CAST(NULL AS REAL)")
+                // .addRoundTrip("real", "3.1415927", REAL, "REAL '3.1415927'") // Overeagerly rounded by MemSQL to 3.14159
                 .execute(getQueryRunner(), trinoCreateAsSelect("trino_test_float"));
-        singlePrecisionFloatingPointTests(memSqlFloatDataType())
-                .execute(getQueryRunner(), memSqlCreateAndInsert("tpch.memsql_test_float"));
-    }
 
-    private static DataTypeTest singlePrecisionFloatingPointTests(DataType<Float> floatType)
-    {
-        // we are not testing Nan/-Infinity/+Infinity as those are not supported by MemSQL
-        return DataTypeTest.create()
-                .addRoundTrip(floatType, 3.14f)
-                // TODO Overeagerly rounded by MemSQL to 3.14159
-                // .addRoundTrip(floatType, 3.1415927f)
-                .addRoundTrip(floatType, null);
+        SqlDataTypeTest.create()
+                .addRoundTrip("float", "3.14", REAL, "REAL '3.14'")
+                .addRoundTrip("float", "10.3e0", REAL, "REAL '10.3e0'")
+                .addRoundTrip("float", "NULL", REAL, "CAST(NULL AS REAL)")
+                // .addRoundTrip("float", "3.1415927", REAL, "REAL '3.1415927'") // Overeagerly rounded by MemSQL to 3.14159
+                .execute(getQueryRunner(), memSqlCreateAndInsert("tpch.memsql_test_float"));
     }
 
     @Test
@@ -851,11 +851,6 @@ private static DataType<String> memSqlJsonDataType(Function<String, String> toLi
         return dataType("json", JSON, toLiteral);
     }
 
-    private static DataType<Float> memSqlFloatDataType()
-    {
-        return dataType("float", REAL, Object::toString);
-    }
-
     private static DataType<Double> memSqlDoubleDataType()
     {
         return dataType("double precision", DOUBLE, Object::toString);