From 1420a67c95f3bc8e183ec8753c6ab59b65cee000 Mon Sep 17 00:00:00 2001 From: kasiafi <30203062+kasiafi@users.noreply.github.com> Date: Tue, 7 Dec 2021 11:49:19 +0100 Subject: [PATCH 1/2] Fix cast from double to varchar: follow the SQL standard in formatting --- .../java/io/trino/type/DoubleOperators.java | 30 +++++++++- .../scalar/TestArrayTransformFunction.java | 2 +- .../operator/scalar/TestLambdaExpression.java | 2 +- .../scalar/TestMapTransformKeysFunction.java | 4 +- .../TestMapTransformValuesFunction.java | 4 +- .../trino/sql/TestExpressionInterpreter.java | 59 +++++++++---------- .../trino/sql/gen/TestExpressionCompiler.java | 3 +- .../rule/TestSimplifyExpressions.java | 8 +-- .../io/trino/type/TestArrayOperators.java | 4 +- .../io/trino/type/TestDoubleOperators.java | 20 +++---- .../java/io/trino/type/TestJsonOperators.java | 4 +- .../java/io/trino/type/TestMapOperators.java | 2 +- 12 files changed, 83 insertions(+), 59 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/type/DoubleOperators.java b/core/trino-main/src/main/java/io/trino/type/DoubleOperators.java index 9f720c0e1877..0f7c081f428d 100644 --- a/core/trino-main/src/main/java/io/trino/type/DoubleOperators.java +++ b/core/trino-main/src/main/java/io/trino/type/DoubleOperators.java @@ -25,6 +25,8 @@ import io.trino.spi.function.SqlType; import io.trino.spi.type.StandardTypes; +import java.text.DecimalFormat; + import static com.google.common.base.Preconditions.checkState; import static io.airlift.slice.Slices.utf8Slice; import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; @@ -53,6 +55,8 @@ public final class DoubleOperators private static final double MIN_BYTE_AS_DOUBLE = -0x1p7; private static final double MAX_BYTE_PLUS_ONE_AS_DOUBLE = 0x1p7; + private static final ThreadLocal FORMAT = ThreadLocal.withInitial(() -> new DecimalFormat("0.0###################E0")); + private DoubleOperators() { } @@ -175,13 +179,35 @@ public static long castToReal(@SqlType(StandardTypes.DOUBLE) double value) @SqlType("varchar(x)") public static Slice castToVarchar(@LiteralParameter("x") long x, @SqlType(StandardTypes.DOUBLE) double value) { - String stringValue = String.valueOf(value); + String stringValue; + + // handle positive and negative 0 + if (value == 0e0) { + if (1e0 / value > 0) { + stringValue = "0E0"; + } + else { + stringValue = "-0E0"; + } + } + else if (Double.isInfinite(value)) { + if (value > 0) { + stringValue = "Infinity"; + } + else { + stringValue = "-Infinity"; + } + } + else { + stringValue = FORMAT.get().format(value); + } + // String is all-ASCII, so String.length() here returns actual code points count if (stringValue.length() <= x) { return utf8Slice(stringValue); } - throw new TrinoException(INVALID_CAST_ARGUMENT, format("Value %s cannot be represented as varchar(%s)", value, x)); + throw new TrinoException(INVALID_CAST_ARGUMENT, format("Value %s (%s) cannot be represented as varchar(%s)", value, stringValue, x)); } @ScalarOperator(SATURATED_FLOOR_CAST) diff --git a/core/trino-main/src/test/java/io/trino/operator/scalar/TestArrayTransformFunction.java b/core/trino-main/src/test/java/io/trino/operator/scalar/TestArrayTransformFunction.java index 5840ac23d08a..d84c71676188 100644 --- a/core/trino-main/src/test/java/io/trino/operator/scalar/TestArrayTransformFunction.java +++ b/core/trino-main/src/test/java/io/trino/operator/scalar/TestArrayTransformFunction.java @@ -84,7 +84,7 @@ public void testTypeCombinations() assertFunction("transform(ARRAY [25.6E0, 27.3E0], x -> CAST(x AS BIGINT))", new ArrayType(BIGINT), ImmutableList.of(26L, 27L)); assertFunction("transform(ARRAY [25.6E0, 27.3E0], x -> x + 1.0E0)", new ArrayType(DOUBLE), ImmutableList.of(26.6, 28.3)); assertFunction("transform(ARRAY [25.6E0, 27.3E0], x -> x = 25.6E0)", new ArrayType(BOOLEAN), ImmutableList.of(true, false)); - assertFunction("transform(ARRAY [25.6E0, 27.3E0], x -> CAST(x AS VARCHAR))", new ArrayType(createUnboundedVarcharType()), ImmutableList.of("25.6", "27.3")); + assertFunction("transform(ARRAY [25.6E0, 27.3E0], x -> CAST(x AS VARCHAR))", new ArrayType(createUnboundedVarcharType()), ImmutableList.of("2.56E1", "2.73E1")); assertFunction( "transform(ARRAY [25.6E0, 27.3E0], x -> MAP(ARRAY[x + 1], ARRAY[true]))", new ArrayType(mapType(DOUBLE, BOOLEAN)), diff --git a/core/trino-main/src/test/java/io/trino/operator/scalar/TestLambdaExpression.java b/core/trino-main/src/test/java/io/trino/operator/scalar/TestLambdaExpression.java index 468842f3753a..e527e6e73f7e 100644 --- a/core/trino-main/src/test/java/io/trino/operator/scalar/TestLambdaExpression.java +++ b/core/trino-main/src/test/java/io/trino/operator/scalar/TestLambdaExpression.java @@ -143,7 +143,7 @@ public void testTypeCombinations() assertFunction("apply(25.6E0, x -> CAST(x AS BIGINT))", BIGINT, 26L); assertFunction("apply(25.6E0, x -> x + 1.0E0)", DOUBLE, 26.6); assertFunction("apply(25.6E0, x -> x = 25.6E0)", BOOLEAN, true); - assertFunction("apply(25.6E0, x -> CAST(x AS VARCHAR))", createUnboundedVarcharType(), "25.6"); + assertFunction("apply(25.6E0, x -> CAST(x AS VARCHAR))", createUnboundedVarcharType(), "2.56E1"); assertFunction("apply(25.6E0, x -> MAP(ARRAY[x + 1], ARRAY[true]))", mapType(DOUBLE, BOOLEAN), ImmutableMap.of(26.6, true)); assertFunction("apply(true, x -> if(x, 25, 26))", INTEGER, 25); diff --git a/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformKeysFunction.java b/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformKeysFunction.java index 4790daff63fe..23694ce1135f 100644 --- a/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformKeysFunction.java +++ b/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformKeysFunction.java @@ -160,11 +160,11 @@ public void testTypeCombinations() assertFunction( "transform_keys(map(ARRAY [25.5E0, 26.5E0, 27.5E0], ARRAY ['abc', 'def', 'xyz']), (k, v) -> CAST(k AS VARCHAR) || substr(v, 1, 1))", mapType(VARCHAR, createVarcharType(3)), - ImmutableMap.of("25.5a", "abc", "26.5d", "def", "27.5x", "xyz")); + ImmutableMap.of("2.55E1a", "abc", "2.65E1d", "def", "2.75E1x", "xyz")); assertFunction( "transform_keys(map(ARRAY [25.5E0, 26.5E0], ARRAY [ARRAY ['a'], ARRAY ['b']]), (k, v) -> ARRAY [CAST(k AS VARCHAR)] || v)", mapType(new ArrayType(VARCHAR), new ArrayType(createVarcharType(1))), - ImmutableMap.of(ImmutableList.of("25.5", "a"), ImmutableList.of("a"), ImmutableList.of("26.5", "b"), ImmutableList.of("b"))); + ImmutableMap.of(ImmutableList.of("2.55E1", "a"), ImmutableList.of("a"), ImmutableList.of("2.65E1", "b"), ImmutableList.of("b"))); assertFunction( "transform_keys(map(ARRAY [true, false], ARRAY [25, 26]), (k, v) -> if(k, 2 * v, 3 * v))", diff --git a/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformValuesFunction.java b/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformValuesFunction.java index 19d13f43d5ac..833af13578fa 100644 --- a/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformValuesFunction.java +++ b/core/trino-main/src/test/java/io/trino/operator/scalar/TestMapTransformValuesFunction.java @@ -102,7 +102,7 @@ public void testBasic() assertFunction( "transform_values(map(ARRAY[1, 2, 3], ARRAY [1.0E0, 1.4E0, 1.7E0]), (k, v) -> map(ARRAY[1, 2, 3], ARRAY['one', 'two', 'three'])[k] || '_' || CAST(v AS VARCHAR))", mapType(INTEGER, VARCHAR), - ImmutableMap.of(1, "one_1.0", 2, "two_1.4", 3, "three_1.7")); + ImmutableMap.of(1, "one_1.0E0", 2, "two_1.4E0", 3, "three_1.7E0")); assertFunction( "transform_values(map(ARRAY[1, 2], ARRAY [TIMESTAMP '2020-05-10 12:34:56.123456789', TIMESTAMP '2010-05-10 12:34:56.123456789']), (k, v) -> date_add('year', 1, v))", @@ -183,7 +183,7 @@ public void testTypeCombinations() assertFunction( "transform_values(map(ARRAY ['s0', 's1', 's2'], ARRAY [25.5E0, 26.5E0, 27.5E0]), (k, v) -> k || ':' || CAST(v as VARCHAR))", mapType(createVarcharType(2), VARCHAR), - ImmutableMap.of("s0", "s0:25.5", "s1", "s1:26.5", "s2", "s2:27.5")); + ImmutableMap.of("s0", "s0:2.55E1", "s1", "s1:2.65E1", "s2", "s2:2.75E1")); assertFunction( "transform_values(map(ARRAY ['s0', 's2'], ARRAY [false, true]), (k, v) -> if(v, k, CAST(v AS VARCHAR)))", mapType(createVarcharType(2), VARCHAR), diff --git a/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java b/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java index 9e766646e332..08e02b9a14f0 100644 --- a/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java +++ b/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java @@ -600,10 +600,10 @@ public void testCastToString() assertOptimizedEquals("CAST(-12300000000 AS varchar)", "'-12300000000'"); // double - assertOptimizedEquals("CAST(123.0E0 AS varchar)", "'123.0'"); - assertOptimizedEquals("CAST(-123.0E0 AS varchar)", "'-123.0'"); - assertOptimizedEquals("CAST(123.456E0 AS varchar)", "'123.456'"); - assertOptimizedEquals("CAST(-123.456E0 AS varchar)", "'-123.456'"); + assertOptimizedEquals("CAST(123.0E0 AS varchar)", "'1.23E2'"); + assertOptimizedEquals("CAST(-123.0E0 AS varchar)", "'-1.23E2'"); + assertOptimizedEquals("CAST(123.456E0 AS varchar)", "'1.23456E2'"); + assertOptimizedEquals("CAST(-123.456E0 AS varchar)", "'-1.23456E2'"); // boolean assertOptimizedEquals("CAST(true AS varchar)", "'true'"); @@ -729,51 +729,48 @@ public void testCastDoubleToBoundedVarchar() assertEvaluatedEquals("CAST(DOUBLE 'Infinity' AS varchar(8))", "'Infinity'"); assertEvaluatedEquals("CAST(DOUBLE 'Infinity' AS varchar(50))", "'Infinity'"); - // incorrect behavior: the string representation is not compliant with the SQL standard - assertEvaluatedEquals("CAST(0e0 AS varchar(3))", "'0.0'"); - assertEvaluatedEquals("CAST(DOUBLE '0' AS varchar(3))", "'0.0'"); - assertEvaluatedEquals("CAST(DOUBLE '-0' AS varchar(4))", "'-0.0'"); - assertEvaluatedEquals("CAST(DOUBLE '0' AS varchar(50))", "'0.0'"); + assertEvaluatedEquals("CAST(0e0 AS varchar(3))", "'0E0'"); + assertEvaluatedEquals("CAST(DOUBLE '0' AS varchar(3))", "'0E0'"); + assertEvaluatedEquals("CAST(DOUBLE '-0' AS varchar(4))", "'-0E0'"); + assertEvaluatedEquals("CAST(DOUBLE '0' AS varchar(50))", "'0E0'"); - assertEvaluatedEquals("CAST(12e0 AS varchar(4))", "'12.0'"); - assertEvaluatedEquals("CAST(12e2 AS varchar(6))", "'1200.0'"); - assertEvaluatedEquals("CAST(12e-2 AS varchar(4))", "'0.12'"); + assertEvaluatedEquals("CAST(12e0 AS varchar(5))", "'1.2E1'"); + assertEvaluatedEquals("CAST(12e2 AS varchar(6))", "'1.2E3'"); + assertEvaluatedEquals("CAST(12e-2 AS varchar(6))", "'1.2E-1'"); - assertEvaluatedEquals("CAST(12e0 AS varchar(50))", "'12.0'"); - assertEvaluatedEquals("CAST(12e2 AS varchar(50))", "'1200.0'"); - assertEvaluatedEquals("CAST(12e-2 AS varchar(50))", "'0.12'"); + assertEvaluatedEquals("CAST(12e0 AS varchar(50))", "'1.2E1'"); + assertEvaluatedEquals("CAST(12e2 AS varchar(50))", "'1.2E3'"); + assertEvaluatedEquals("CAST(12e-2 AS varchar(50))", "'1.2E-1'"); - assertEvaluatedEquals("CAST(-12e0 AS varchar(5))", "'-12.0'"); - assertEvaluatedEquals("CAST(-12e2 AS varchar(7))", "'-1200.0'"); - assertEvaluatedEquals("CAST(-12e-2 AS varchar(5))", "'-0.12'"); + assertEvaluatedEquals("CAST(-12e0 AS varchar(6))", "'-1.2E1'"); + assertEvaluatedEquals("CAST(-12e2 AS varchar(6))", "'-1.2E3'"); + assertEvaluatedEquals("CAST(-12e-2 AS varchar(7))", "'-1.2E-1'"); - assertEvaluatedEquals("CAST(-12e0 AS varchar(50))", "'-12.0'"); - assertEvaluatedEquals("CAST(-12e2 AS varchar(50))", "'-1200.0'"); - assertEvaluatedEquals("CAST(-12e-2 AS varchar(50))", "'-0.12'"); + assertEvaluatedEquals("CAST(-12e0 AS varchar(50))", "'-1.2E1'"); + assertEvaluatedEquals("CAST(-12e2 AS varchar(50))", "'-1.2E3'"); + assertEvaluatedEquals("CAST(-12e-2 AS varchar(50))", "'-1.2E-1'"); - // the string representation is compliant with the SQL standard assertEvaluatedEquals("CAST(12345678.9e0 AS varchar(12))", "'1.23456789E7'"); assertEvaluatedEquals("CAST(0.00001e0 AS varchar(6))", "'1.0E-5'"); - // the result value does not fit in the type (also, it is not compliant with the SQL standard) + // the result value does not fit in the type assertTrinoExceptionThrownBy(() -> evaluate("CAST(12e0 AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value 12.0 cannot be represented as varchar(1)"); + .hasMessage("Value 12.0 (1.2E1) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(-12e2 AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value -1200.0 cannot be represented as varchar(1)"); + .hasMessage("Value -1200.0 (-1.2E3) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(0e0 AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value 0.0 cannot be represented as varchar(1)"); + .hasMessage("Value 0.0 (0E0) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(0e0 / 0e0 AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value NaN cannot be represented as varchar(1)"); + .hasMessage("Value NaN (NaN) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(DOUBLE 'Infinity' AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value Infinity cannot be represented as varchar(1)"); - assertTrinoExceptionThrownBy(() -> evaluate("CAST(1200000e0 AS varchar(5))")) - .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value 1200000.0 cannot be represented as varchar(5)"); + .hasMessage("Value Infinity (Infinity) cannot be represented as varchar(1)"); + + assertEvaluatedEquals("CAST(1200000e0 AS varchar(5))", "'1.2E6'"); } @Test diff --git a/core/trino-main/src/test/java/io/trino/sql/gen/TestExpressionCompiler.java b/core/trino-main/src/test/java/io/trino/sql/gen/TestExpressionCompiler.java index a3f8e2a584c1..594ef115a5c9 100644 --- a/core/trino-main/src/test/java/io/trino/sql/gen/TestExpressionCompiler.java +++ b/core/trino-main/src/test/java/io/trino/sql/gen/TestExpressionCompiler.java @@ -832,13 +832,14 @@ public void testCast() assertExecute(generateExpression("cast(%s as varchar)", value), VARCHAR, value == null ? null : String.valueOf(value)); } + DecimalFormat doubleFormat = new DecimalFormat("0.0###################E0"); for (Double value : doubleLefts) { assertExecute(generateExpression("cast(%s as boolean)", value), BOOLEAN, value == null ? null : (value != 0.0 ? true : false)); if (value == null || (value >= Long.MIN_VALUE && value < Long.MAX_VALUE)) { assertExecute(generateExpression("cast(%s as bigint)", value), BIGINT, value == null ? null : value.longValue()); } assertExecute(generateExpression("cast(%s as double)", value), DOUBLE, value == null ? null : value); - assertExecute(generateExpression("cast(%s as varchar)", value), VARCHAR, value == null ? null : String.valueOf(value)); + assertExecute(generateExpression("cast(%s as varchar)", value), VARCHAR, value == null ? null : doubleFormat.format(value)); } assertExecute("cast('true' as boolean)", BOOLEAN, true); diff --git a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java index 123e80ad1d28..2a3320e1a0d5 100644 --- a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java +++ b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java @@ -208,12 +208,12 @@ public void testCastLongDecimalToBoundedVarchar() public void testCastDoubleToBoundedVarchar() { // the varchar type length is enough to contain the number's representation - assertSimplifies("CAST(0e0 AS varchar(3))", "'0.0'"); - assertSimplifies("CAST(-0e0 AS varchar(4))", "'-0.0'"); + assertSimplifies("CAST(0e0 AS varchar(3))", "'0E0'"); + assertSimplifies("CAST(-0e0 AS varchar(4))", "'-0E0'"); assertSimplifies("CAST(0e0 / 0e0 AS varchar(3))", "'NaN'"); assertSimplifies("CAST(DOUBLE 'Infinity' AS varchar(8))", "'Infinity'"); - assertSimplifies("CAST(12e2 AS varchar(6))", "'1200.0'"); - assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1200.0' AS varchar(50))"); + assertSimplifies("CAST(12e2 AS varchar(5))", "'1.2E3'"); + assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1.2E3' AS varchar(50))"); // cast from double to varchar fails, so the expression is not modified assertSimplifies("CAST(12e2 AS varchar(3))", "CAST(12e2 AS varchar(3))"); diff --git a/core/trino-main/src/test/java/io/trino/type/TestArrayOperators.java b/core/trino-main/src/test/java/io/trino/type/TestArrayOperators.java index a1e6f8835f2d..53c13cce4045 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestArrayOperators.java +++ b/core/trino-main/src/test/java/io/trino/type/TestArrayOperators.java @@ -271,7 +271,7 @@ public void testJsonToArray() // varchar, json assertFunction("CAST(JSON '[true, false, 12, 12.3, \"puppies\", \"kittens\", \"null\", \"\", null]' AS ARRAY)", new ArrayType(VARCHAR), - asList("true", "false", "12", "12.3", "puppies", "kittens", "null", "", null)); + asList("true", "false", "12", "1.23E1", "puppies", "kittens", "null", "", null)); assertFunction("CAST(JSON '[5, 3.14, [1, 2, 3], \"e\", {\"a\": \"b\"}, null, \"null\", [null]]' AS ARRAY)", new ArrayType(JSON), ImmutableList.of("5", "3.14", "[1,2,3]", "\"e\"", "{\"a\":\"b\"}", "null", "\"null\"", "[null]")); @@ -560,7 +560,7 @@ public void testArrayJoin() assertFunction("ARRAY_JOIN(ARRAY [1.0, 2.1, 3.3], 'x')", VARCHAR, "1.0x2.1x3.3"); assertFunction("ARRAY_JOIN(ARRAY [1.0, 2.100, 3.3], 'x')", VARCHAR, "1.000x2.100x3.300"); assertFunction("ARRAY_JOIN(ARRAY [1.0, 2.100, NULL], 'x', 'N/A')", VARCHAR, "1.000x2.100xN/A"); - assertFunction("ARRAY_JOIN(ARRAY [1.0, DOUBLE '002.100', 3.3], 'x')", VARCHAR, "1.0x2.1x3.3"); + assertFunction("ARRAY_JOIN(ARRAY [1.0, DOUBLE '002.100', 3.3], 'x')", VARCHAR, "1.0E0x2.1E0x3.3E0"); assertInvalidFunction("ARRAY_JOIN(ARRAY [ARRAY [1], ARRAY [2]], '-')", FUNCTION_NOT_FOUND); assertInvalidFunction("ARRAY_JOIN(ARRAY [MAP(ARRAY [1], ARRAY [2])], '-')", FUNCTION_NOT_FOUND); diff --git a/core/trino-main/src/test/java/io/trino/type/TestDoubleOperators.java b/core/trino-main/src/test/java/io/trino/type/TestDoubleOperators.java index 400f1566dca4..13d85a83b2bd 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestDoubleOperators.java +++ b/core/trino-main/src/test/java/io/trino/type/TestDoubleOperators.java @@ -227,20 +227,20 @@ public void testBetween() @Test public void testCastToVarchar() { - assertFunction("cast(37.7E0 as varchar)", VARCHAR, "37.7"); - assertFunction("cast(17.1E0 as varchar)", VARCHAR, "17.1"); + assertFunction("cast(37.7E0 as varchar)", VARCHAR, "3.77E1"); + assertFunction("cast(17.1E0 as varchar)", VARCHAR, "1.71E1"); assertFunction("cast(0E0/0E0 as varchar)", VARCHAR, "NaN"); - assertFunction("cast(12e2 as varchar(6))", createVarcharType(6), "1200.0"); - assertFunction("cast(12e2 as varchar(50))", createVarcharType(50), "1200.0"); + assertFunction("cast(12e2 as varchar(6))", createVarcharType(6), "1.2E3"); + assertFunction("cast(12e2 as varchar(50))", createVarcharType(50), "1.2E3"); assertFunction("cast(12345678.9e0 as varchar(50))", createVarcharType(50), "1.23456789E7"); assertFunction("cast(DOUBLE 'NaN' as varchar(3))", createVarcharType(3), "NaN"); assertFunction("cast(DOUBLE 'Infinity' as varchar(50))", createVarcharType(50), "Infinity"); - assertInvalidCast("cast(12e2 as varchar(5))", "Value 1200.0 cannot be represented as varchar(5)"); - assertInvalidCast("cast(12e2 as varchar(4))", "Value 1200.0 cannot be represented as varchar(4)"); - assertInvalidCast("cast(0e0 as varchar(2))", "Value 0.0 cannot be represented as varchar(2)"); - assertInvalidCast("cast(-0e0 as varchar(3))", "Value -0.0 cannot be represented as varchar(3)"); - assertInvalidCast("cast(0e0 / 0e0 as varchar(2))", "Value NaN cannot be represented as varchar(2)"); - assertInvalidCast("cast(DOUBLE 'Infinity' as varchar(7))", "Value Infinity cannot be represented as varchar(7)"); + assertFunction("cast(12e2 as varchar(5))", createVarcharType(5), "1.2E3"); + assertInvalidCast("cast(12e2 as varchar(4))", "Value 1200.0 (1.2E3) cannot be represented as varchar(4)"); + assertInvalidCast("cast(0e0 as varchar(2))", "Value 0.0 (0E0) cannot be represented as varchar(2)"); + assertInvalidCast("cast(-0e0 as varchar(3))", "Value -0.0 (-0E0) cannot be represented as varchar(3)"); + assertInvalidCast("cast(0e0 / 0e0 as varchar(2))", "Value NaN (NaN) cannot be represented as varchar(2)"); + assertInvalidCast("cast(DOUBLE 'Infinity' as varchar(7))", "Value Infinity (Infinity) cannot be represented as varchar(7)"); } @Test diff --git a/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java b/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java index 220f7f1189d4..ce15d0cfb1bc 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java +++ b/core/trino-main/src/test/java/io/trino/type/TestJsonOperators.java @@ -328,8 +328,8 @@ public void testCastToVarchar() assertFunction("cast(JSON 'null' as VARCHAR)", VARCHAR, null); assertFunction("cast(JSON '128' as VARCHAR)", VARCHAR, "128"); assertFunction("cast(JSON '12345678901234567890' as VARCHAR)", VARCHAR, "12345678901234567890"); // overflow, no loss of precision - assertFunction("cast(JSON '128.9' as VARCHAR)", VARCHAR, "128.9"); - assertFunction("cast(JSON '1e-324' as VARCHAR)", VARCHAR, "0.0"); // smaller than minimum subnormal positive + assertFunction("cast(JSON '128.9' as VARCHAR)", VARCHAR, "1.289E2"); + assertFunction("cast(JSON '1e-324' as VARCHAR)", VARCHAR, "0E0"); // smaller than minimum subnormal positive assertFunction("cast(JSON '1e309' as VARCHAR)", VARCHAR, "Infinity"); // overflow assertFunction("cast(JSON '-1e309' as VARCHAR)", VARCHAR, "-Infinity"); // underflow assertFunction("cast(JSON 'true' as VARCHAR)", VARCHAR, "true"); diff --git a/core/trino-main/src/test/java/io/trino/type/TestMapOperators.java b/core/trino-main/src/test/java/io/trino/type/TestMapOperators.java index bb2b6b4e22a4..0f473e7ce659 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestMapOperators.java +++ b/core/trino-main/src/test/java/io/trino/type/TestMapOperators.java @@ -374,7 +374,7 @@ public void testJsonToMap() mapType(BIGINT, VARCHAR), asMap( ImmutableList.of(1L, 2L, 3L, 5L, 8L, 13L, 21L, 34L, 55L), - asList("true", "false", "12", "12.3", "puppies", "kittens", "null", "", null))); + asList("true", "false", "12", "1.23E1", "puppies", "kittens", "null", "", null))); assertFunction("CAST(JSON '{\"k1\": 5, \"k2\": 3.14, \"k3\":[1, 2, 3], \"k4\":\"e\", \"k5\":{\"a\": \"b\"}, \"k6\":null, \"k7\":\"null\", \"k8\":[null]}' AS MAP)", mapType(VARCHAR, JSON), From ce125d01272626579138b1f71afd6cfed0ee718d Mon Sep 17 00:00:00 2001 From: kasiafi <30203062+kasiafi@users.noreply.github.com> Date: Thu, 20 Jan 2022 21:05:57 +0100 Subject: [PATCH 2/2] Fix cast from real to varchar: follow the SQL standard in formatting --- .../java/io/trino/type/RealOperators.java | 31 ++++++++++- .../trino/sql/TestExpressionInterpreter.java | 53 +++++++++---------- .../rule/TestSimplifyExpressions.java | 8 +-- .../java/io/trino/type/TestRealOperators.java | 22 ++++---- 4 files changed, 69 insertions(+), 45 deletions(-) diff --git a/core/trino-main/src/main/java/io/trino/type/RealOperators.java b/core/trino-main/src/main/java/io/trino/type/RealOperators.java index 2c1bb0a51bca..c34adbc12f6d 100644 --- a/core/trino-main/src/main/java/io/trino/type/RealOperators.java +++ b/core/trino-main/src/main/java/io/trino/type/RealOperators.java @@ -25,6 +25,8 @@ import io.trino.spi.function.SqlType; import io.trino.spi.type.StandardTypes; +import java.text.DecimalFormat; + import static io.airlift.slice.Slices.utf8Slice; import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; import static io.trino.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; @@ -49,6 +51,8 @@ public final class RealOperators private static final float MIN_BYTE_AS_FLOAT = -0x1p7f; private static final float MAX_BYTE_PLUS_ONE_AS_FLOAT = 0x1p7f; + private static final ThreadLocal FORMAT = ThreadLocal.withInitial(() -> new DecimalFormat("0.0#####E0")); + private RealOperators() { } @@ -100,13 +104,36 @@ public static long negate(@SqlType(StandardTypes.REAL) long value) @SqlType("varchar(x)") public static Slice castToVarchar(@LiteralParameter("x") long x, @SqlType(StandardTypes.REAL) long value) { - String stringValue = String.valueOf(intBitsToFloat((int) value)); + float floatValue = intBitsToFloat((int) value); + String stringValue; + + // handle positive and negative 0 + if (floatValue == 0.0f) { + if (1.0f / floatValue > 0) { + stringValue = "0E0"; + } + else { + stringValue = "-0E0"; + } + } + else if (Float.isInfinite(floatValue)) { + if (floatValue > 0) { + stringValue = "Infinity"; + } + else { + stringValue = "-Infinity"; + } + } + else { + stringValue = FORMAT.get().format(Double.parseDouble(Float.toString(floatValue))); + } + // String is all-ASCII, so String.length() here returns actual code points count if (stringValue.length() <= x) { return utf8Slice(stringValue); } - throw new TrinoException(INVALID_CAST_ARGUMENT, format("Value %s cannot be represented as varchar(%s)", stringValue, x)); + throw new TrinoException(INVALID_CAST_ARGUMENT, format("Value %s (%s) cannot be represented as varchar(%s)", floatValue, stringValue, x)); } @ScalarOperator(CAST) diff --git a/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java b/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java index 08e02b9a14f0..16dd2e87ee6b 100644 --- a/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java +++ b/core/trino-main/src/test/java/io/trino/sql/TestExpressionInterpreter.java @@ -784,50 +784,47 @@ public void testCastRealToBoundedVarchar() assertEvaluatedEquals("CAST(REAL 'Infinity' AS varchar(8))", "'Infinity'"); assertEvaluatedEquals("CAST(REAL 'Infinity' AS varchar(50))", "'Infinity'"); - // incorrect behavior: the string representation is not compliant with the SQL standard - assertEvaluatedEquals("CAST(REAL '0' AS varchar(3))", "'0.0'"); - assertEvaluatedEquals("CAST(REAL '-0' AS varchar(4))", "'-0.0'"); - assertEvaluatedEquals("CAST(REAL '0' AS varchar(50))", "'0.0'"); + assertEvaluatedEquals("CAST(REAL '0' AS varchar(3))", "'0E0'"); + assertEvaluatedEquals("CAST(REAL '-0' AS varchar(4))", "'-0E0'"); + assertEvaluatedEquals("CAST(REAL '0' AS varchar(50))", "'0E0'"); - assertEvaluatedEquals("CAST(REAL '12' AS varchar(4))", "'12.0'"); - assertEvaluatedEquals("CAST(REAL '12e2' AS varchar(6))", "'1200.0'"); - assertEvaluatedEquals("CAST(REAL '12e-2' AS varchar(4))", "'0.12'"); + assertEvaluatedEquals("CAST(REAL '12' AS varchar(5))", "'1.2E1'"); + assertEvaluatedEquals("CAST(REAL '12e2' AS varchar(5))", "'1.2E3'"); + assertEvaluatedEquals("CAST(REAL '12e-2' AS varchar(6))", "'1.2E-1'"); - assertEvaluatedEquals("CAST(REAL '12' AS varchar(50))", "'12.0'"); - assertEvaluatedEquals("CAST(REAL '12e2' AS varchar(50))", "'1200.0'"); - assertEvaluatedEquals("CAST(REAL '12e-2' AS varchar(50))", "'0.12'"); + assertEvaluatedEquals("CAST(REAL '12' AS varchar(50))", "'1.2E1'"); + assertEvaluatedEquals("CAST(REAL '12e2' AS varchar(50))", "'1.2E3'"); + assertEvaluatedEquals("CAST(REAL '12e-2' AS varchar(50))", "'1.2E-1'"); - assertEvaluatedEquals("CAST(REAL '-12' AS varchar(5))", "'-12.0'"); - assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(7))", "'-1200.0'"); - assertEvaluatedEquals("CAST(REAL '-12e-2' AS varchar(5))", "'-0.12'"); + assertEvaluatedEquals("CAST(REAL '-12' AS varchar(6))", "'-1.2E1'"); + assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(6))", "'-1.2E3'"); + assertEvaluatedEquals("CAST(REAL '-12e-2' AS varchar(7))", "'-1.2E-1'"); - assertEvaluatedEquals("CAST(REAL '-12' AS varchar(50))", "'-12.0'"); - assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(50))", "'-1200.0'"); - assertEvaluatedEquals("CAST(REAL '-12e-2' AS varchar(50))", "'-0.12'"); + assertEvaluatedEquals("CAST(REAL '-12' AS varchar(50))", "'-1.2E1'"); + assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(50))", "'-1.2E3'"); + assertEvaluatedEquals("CAST(REAL '-12e-2' AS varchar(50))", "'-1.2E-1'"); - // the string representation is compliant with the SQL standard - assertEvaluatedEquals("CAST(REAL '12345678.9e0' AS varchar(12))", "'1.2345679E7'"); - assertEvaluatedEquals("CAST(REAL '0.00001e0' AS varchar(6))", "'1.0E-5'"); + assertEvaluatedEquals("CAST(REAL '12345678.9e0' AS varchar(12))", "'1.234568E7'"); + assertEvaluatedEquals("CAST(REAL '0.00001e0' AS varchar(12))", "'1.0E-5'"); - // the result value does not fit in the type (also, it is not compliant with the SQL standard) + // the result value does not fit in the type assertTrinoExceptionThrownBy(() -> evaluate("CAST(REAL '12' AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value 12.0 cannot be represented as varchar(1)"); + .hasMessage("Value 12.0 (1.2E1) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(REAL '-12e2' AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value -1200.0 cannot be represented as varchar(1)"); + .hasMessage("Value -1200.0 (-1.2E3) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(REAL '0' AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value 0.0 cannot be represented as varchar(1)"); + .hasMessage("Value 0.0 (0E0) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(REAL '0e0' / REAL '0e0' AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value NaN cannot be represented as varchar(1)"); + .hasMessage("Value NaN (NaN) cannot be represented as varchar(1)"); assertTrinoExceptionThrownBy(() -> evaluate("CAST(REAL 'Infinity' AS varchar(1))")) .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value Infinity cannot be represented as varchar(1)"); - assertTrinoExceptionThrownBy(() -> evaluate("CAST(REAL '1200000' AS varchar(5))")) - .hasErrorCode(INVALID_CAST_ARGUMENT) - .hasMessage("Value 1200000.0 cannot be represented as varchar(5)"); + .hasMessage("Value Infinity (Infinity) cannot be represented as varchar(1)"); + + assertEvaluatedEquals("CAST(REAL '1200000' AS varchar(5))", "'1.2E6'"); } @Test diff --git a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java index 2a3320e1a0d5..45bc25e4b073 100644 --- a/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java +++ b/core/trino-main/src/test/java/io/trino/sql/planner/iterative/rule/TestSimplifyExpressions.java @@ -227,12 +227,12 @@ public void testCastDoubleToBoundedVarchar() public void testCastRealToBoundedVarchar() { // the varchar type length is enough to contain the number's representation - assertSimplifies("CAST(REAL '0e0' AS varchar(3))", "'0.0'"); - assertSimplifies("CAST(REAL '-0e0' AS varchar(4))", "'-0.0'"); + assertSimplifies("CAST(REAL '0e0' AS varchar(3))", "'0E0'"); + assertSimplifies("CAST(REAL '-0e0' AS varchar(4))", "'-0E0'"); assertSimplifies("CAST(REAL '0e0' / REAL '0e0' AS varchar(3))", "'NaN'"); assertSimplifies("CAST(REAL 'Infinity' AS varchar(8))", "'Infinity'"); - assertSimplifies("CAST(REAL '12e2' AS varchar(6))", "'1200.0'"); - assertSimplifies("CAST(REAL '-12e2' AS varchar(50))", "CAST('-1200.0' AS varchar(50))"); + assertSimplifies("CAST(REAL '12e2' AS varchar(5))", "'1.2E3'"); + assertSimplifies("CAST(REAL '-12e2' AS varchar(50))", "CAST('-1.2E3' AS varchar(50))"); // cast from real to varchar fails, so the expression is not modified assertSimplifies("CAST(REAL '12e2' AS varchar(3))", "CAST(REAL '12e2' AS varchar(3))"); diff --git a/core/trino-main/src/test/java/io/trino/type/TestRealOperators.java b/core/trino-main/src/test/java/io/trino/type/TestRealOperators.java index 015553c3c0e9..18874e5f1f3d 100644 --- a/core/trino-main/src/test/java/io/trino/type/TestRealOperators.java +++ b/core/trino-main/src/test/java/io/trino/type/TestRealOperators.java @@ -218,21 +218,21 @@ public void testBetween() @Test public void testCastToVarchar() { - assertFunction("CAST(REAL '754.1985' as VARCHAR)", VARCHAR, "754.1985"); - assertFunction("CAST(REAL '-754.2008' as VARCHAR)", VARCHAR, "-754.2008"); + assertFunction("CAST(REAL '754.1985' as VARCHAR)", VARCHAR, "7.541985E2"); + assertFunction("CAST(REAL '-754.2008' as VARCHAR)", VARCHAR, "-7.542008E2"); assertFunction("CAST(REAL 'Infinity' as VARCHAR)", VARCHAR, "Infinity"); assertFunction("CAST(REAL '0.0' / REAL '0.0' as VARCHAR)", VARCHAR, "NaN"); - assertFunction("cast(REAL '12e2' as varchar(6))", createVarcharType(6), "1200.0"); - assertFunction("cast(REAL '12e2' as varchar(50))", createVarcharType(50), "1200.0"); - assertFunction("cast(REAL '12345678.9e0' as varchar(50))", createVarcharType(50), "1.2345679E7"); + assertFunction("cast(REAL '12e2' as varchar(6))", createVarcharType(6), "1.2E3"); + assertFunction("cast(REAL '12e2' as varchar(50))", createVarcharType(50), "1.2E3"); + assertFunction("cast(REAL '12345678.9e0' as varchar(50))", createVarcharType(50), "1.234568E7"); assertFunction("cast(REAL 'NaN' as varchar(3))", createVarcharType(3), "NaN"); assertFunction("cast(REAL 'Infinity' as varchar(50))", createVarcharType(50), "Infinity"); - assertInvalidCast("cast(REAL '12e2' as varchar(5))", "Value 1200.0 cannot be represented as varchar(5)"); - assertInvalidCast("cast(REAL '12e2' as varchar(4))", "Value 1200.0 cannot be represented as varchar(4)"); - assertInvalidCast("cast(REAL '0e0' as varchar(2))", "Value 0.0 cannot be represented as varchar(2)"); - assertInvalidCast("cast(REAL '-0e0' as varchar(3))", "Value -0.0 cannot be represented as varchar(3)"); - assertInvalidCast("cast(REAL '0e0' / REAL '0e0' as varchar(2))", "Value NaN cannot be represented as varchar(2)"); - assertInvalidCast("cast(REAL 'Infinity' as varchar(7))", "Value Infinity cannot be represented as varchar(7)"); + assertFunction("cast(REAL '12e2' as varchar(5))", createVarcharType(5), "1.2E3"); + assertInvalidCast("cast(REAL '12e2' as varchar(4))", "Value 1200.0 (1.2E3) cannot be represented as varchar(4)"); + assertInvalidCast("cast(REAL '0e0' as varchar(2))", "Value 0.0 (0E0) cannot be represented as varchar(2)"); + assertInvalidCast("cast(REAL '-0e0' as varchar(3))", "Value -0.0 (-0E0) cannot be represented as varchar(3)"); + assertInvalidCast("cast(REAL '0e0' / REAL '0e0' as varchar(2))", "Value NaN (NaN) cannot be represented as varchar(2)"); + assertInvalidCast("cast(REAL 'Infinity' as varchar(7))", "Value Infinity (Infinity) cannot be represented as varchar(7)"); } @Test