From 65ecdd6553c1cf476a6b9b8bc0ffd50678cdfdcd Mon Sep 17 00:00:00 2001 From: v-jizhang Date: Wed, 2 Feb 2022 12:28:18 -0800 Subject: [PATCH] Fix cast from real to varchar: follow the SQL standard in formatting Cherry-pick of https://github.com/trinodb/trino/pull/10657/commits/ce125d01272626579138b1f71afd6cfed0ee718d Co-authored-by: kasiafi <30203062+kasiafi@users.noreply.github.com> --- .../facebook/presto/type/RealOperators.java | 34 +++++++- .../presto/sql/TestExpressionInterpreter.java | 78 ++++++++++--------- .../rule/TestSimplifyExpressions.java | 49 +++++++----- .../presto/type/TestRealOperators.java | 22 +++--- 4 files changed, 112 insertions(+), 71 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/type/RealOperators.java b/presto-main/src/main/java/com/facebook/presto/type/RealOperators.java index dc4447be5cb4d..4f43482ff6380 100644 --- a/presto-main/src/main/java/com/facebook/presto/type/RealOperators.java +++ b/presto-main/src/main/java/com/facebook/presto/type/RealOperators.java @@ -31,6 +31,8 @@ import io.airlift.slice.Slice; import io.airlift.slice.XxHash64; +import java.text.DecimalFormat; + import static com.facebook.presto.common.function.OperatorType.ADD; import static com.facebook.presto.common.function.OperatorType.BETWEEN; import static com.facebook.presto.common.function.OperatorType.CAST; @@ -72,6 +74,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() { } @@ -189,13 +193,39 @@ public static long xxHash64(@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 if (Float.isNaN(floatValue)) { + stringValue = "NaN"; + } + 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 PrestoException(INVALID_CAST_ARGUMENT, format("Value %s cannot be represented as varchar(%s)", stringValue, x)); + throw new PrestoException(INVALID_CAST_ARGUMENT, format("Value %s (%s) cannot be represented as varchar(%s)", floatValue, stringValue, x)); } @ScalarOperator(CAST) diff --git a/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java b/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java index 1c45fea686853..eeb666678c28f 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/TestExpressionInterpreter.java @@ -588,18 +588,21 @@ public void testCastToString() } @Test - public void testCastBigintToBoundedVarchar() { + public void testCastBigintToBoundedVarchar() + { assertEvaluatedEquals("CAST(12300000000 AS varchar(11))", "'12300000000'"); assertEvaluatedEquals("CAST(12300000000 AS varchar(50))", "'12300000000'"); try { evaluate("CAST(12300000000 AS varchar(3))", true); fail("Expected to throw an INVALID_CAST_ARGUMENT exception"); - } catch (PrestoException e) { + } + catch (PrestoException e) { try { assertEquals(e.getErrorCode(), INVALID_CAST_ARGUMENT.toErrorCode()); assertEquals(e.getMessage(), "Value 12300000000 cannot be represented as varchar(3)"); - } catch (Throwable failure) { + } + catch (Throwable failure) { failure.addSuppressed(e); throw failure; } @@ -607,11 +610,13 @@ public void testCastBigintToBoundedVarchar() { try { evaluate("CAST(-12300000000 AS varchar(3))", true); - } catch (PrestoException e) { + } + catch (PrestoException e) { try { assertEquals(e.getErrorCode(), INVALID_CAST_ARGUMENT.toErrorCode()); assertEquals(e.getMessage(), "Value -12300000000 cannot be represented as varchar(3)"); - } catch (Throwable failure) { + } + catch (Throwable failure) { failure.addSuppressed(e); throw failure; } @@ -675,38 +680,37 @@ public void testCastRealToBoundedVarchar() 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 '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(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(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(50))", "'-12.0'"); - assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(50))", "'-1200.0'"); - assertEvaluatedEquals("CAST(REAL '-12e-2' AS varchar(50))", "'-0.12'"); - - // 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'"); - - // the result value does not fit in the type (also, it is not compliant with the SQL standard) - assertPrestoExceptionThrownBy("CAST(12e0 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 12.0 cannot be represented as varchar(1)"); - - assertPrestoExceptionThrownBy("CAST(-12e2 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value -1200.0 cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(0e0 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 0.0 cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(0e0 / 0e0 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value NaN cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(DOUBLE 'Infinity' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value Infinity cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(1200000e0 AS varchar(5))", INVALID_CAST_ARGUMENT, "Value 1200000.0 cannot be represented as varchar(5)"); + 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(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))", "'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(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))", "'-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 '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 + assertPrestoExceptionThrownBy("CAST(12e0 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 12.0 (1.2E1) cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(-12e2 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value -1200.0 (-1.2E3) cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(0e0 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 0.0 (0E0) cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(0e0 / 0e0 AS varchar(1))", INVALID_CAST_ARGUMENT, "Value NaN (NaN) cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(DOUBLE 'Infinity' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value Infinity (Infinity) cannot be represented as varchar(1)"); + + assertEvaluatedEquals("CAST(REAL '1200000' AS varchar(5))", "'1.2E6'"); } @Test diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestSimplifyExpressions.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestSimplifyExpressions.java index 1680c5b253555..b486586c9a60b 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestSimplifyExpressions.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestSimplifyExpressions.java @@ -138,7 +138,8 @@ public void testExtractCommonPredicates() } @Test - public void testCastBigintToBoundedVarchar() { + public void testCastBigintToBoundedVarchar() + { // the varchar type length is enough to contain the number's representation assertSimplifies("CAST(12300000000 AS varchar(11))", "'12300000000'"); // The last argument "'-12300000000'" is varchar(12). Need varchar(50) to the following test pass. @@ -148,11 +149,13 @@ public void testCastBigintToBoundedVarchar() { try { assertSimplifies("CAST(12300000000 AS varchar(3))", "CAST(12300000000 AS varchar(3))"); fail("Expected to throw an PrestoException exception"); - } catch (PrestoException e) { + } + catch (PrestoException e) { try { assertEquals(e.getErrorCode(), INVALID_CAST_ARGUMENT.toErrorCode()); assertEquals(e.getMessage(), "Value 12300000000 cannot be represented as varchar(3)"); - } catch (Throwable failure) { + } + catch (Throwable failure) { failure.addSuppressed(e); throw failure; } @@ -160,11 +163,13 @@ public void testCastBigintToBoundedVarchar() { try { assertSimplifies("CAST(-12300000000 AS varchar(3))", "CAST(-12300000000 AS varchar(3))"); - } catch (PrestoException e) { + } + catch (PrestoException e) { try { assertEquals(e.getErrorCode(), INVALID_CAST_ARGUMENT.toErrorCode()); assertEquals(e.getMessage(), "Value -12300000000 cannot be represented as varchar(3)"); - } catch (Throwable failure) { + } + catch (Throwable failure) { failure.addSuppressed(e); throw failure; } @@ -180,33 +185,35 @@ public void testCastDoubleToBoundedVarchar() assertSimplifies("CAST(0e0 / 0e0 AS varchar(3))", "'NaN'"); assertSimplifies("CAST(DOUBLE 'Infinity' AS varchar(8))", "'Infinity'"); assertSimplifies("CAST(12e2 AS varchar(5))", "'1.2E3'"); - assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1.2E3' AS varchar(50))"); + + // The last argument "'-1.2E3'" is varchar(6). Need varchar(50) to the following test pass. + // assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1.2E3' AS varchar(50))", "'-1.2E3'"); /// cast from double to varchar fails - assertPrestoExceptionThrownBy("CAST(12e2 AS varchar(3))", INVALID_CAST_ARGUMENT, "Value 1200.0 cannot be represented as varchar(3)"); - assertPrestoExceptionThrownBy("CAST(-12e2 AS varchar(3))", INVALID_CAST_ARGUMENT, "Value -1200.0 cannot be represented as varchar(3)"); - assertPrestoExceptionThrownBy("CAST(DOUBLE 'NaN' AS varchar(2))", INVALID_CAST_ARGUMENT, "Value NaN cannot be represented as varchar(2)"); - assertPrestoExceptionThrownBy("CAST(DOUBLE 'Infinity' AS varchar(7))", INVALID_CAST_ARGUMENT, "Value Infinity cannot be represented as varchar(7)"); - assertPrestoExceptionThrownBy("CAST(12e2 AS varchar(3)) = '1200.0'", INVALID_CAST_ARGUMENT, "Value 1200.0 cannot be represented as varchar(3)"); + assertPrestoExceptionThrownBy("CAST(12e2 AS varchar(3))", INVALID_CAST_ARGUMENT, "Value 1200.0 (1.2E3) cannot be represented as varchar(3)"); + assertPrestoExceptionThrownBy("CAST(-12e2 AS varchar(3))", INVALID_CAST_ARGUMENT, "Value -1200.0 (-1.2E3) cannot be represented as varchar(3)"); + assertPrestoExceptionThrownBy("CAST(DOUBLE 'NaN' AS varchar(2))", INVALID_CAST_ARGUMENT, "Value NaN (NaN) cannot be represented as varchar(2)"); + assertPrestoExceptionThrownBy("CAST(DOUBLE 'Infinity' AS varchar(7))", INVALID_CAST_ARGUMENT, "Value Infinity (Infinity) cannot be represented as varchar(7)"); + assertPrestoExceptionThrownBy("CAST(12e2 AS varchar(3)) = '1200.0'", INVALID_CAST_ARGUMENT, "Value 1200.0 (1.2E3) cannot be represented as varchar(3)"); } @Test 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))"); - assertSimplifies("CAST(REAL '-12e2' AS varchar(3))", "CAST(REAL '-12e2' AS varchar(3))"); - assertSimplifies("CAST(REAL 'NaN' AS varchar(2))", "CAST(REAL 'NaN' AS varchar(2))"); - assertSimplifies("CAST(REAL 'Infinity' AS varchar(7))", "CAST(REAL 'Infinity' AS varchar(7))"); - assertSimplifies("CAST(REAL '12e2' AS varchar(3)) = '1200.0'", "CAST(REAL '12e2' AS varchar(3)) = '1200.0'"); + // cast from real to varchar fails + assertPrestoExceptionThrownBy("CAST(REAL '12e2' AS varchar(3))", INVALID_CAST_ARGUMENT, "Value 1200.0 (1.2E3) cannot be represented as varchar(3)"); + assertPrestoExceptionThrownBy("CAST(REAL '-12e2' AS varchar(3))", INVALID_CAST_ARGUMENT, "Value -1200.0 (-1.2E3) cannot be represented as varchar(3)"); + assertPrestoExceptionThrownBy("CAST(REAL 'NaN' AS varchar(2))", INVALID_CAST_ARGUMENT, "Value NaN (NaN) cannot be represented as varchar(2)"); + assertPrestoExceptionThrownBy("CAST(REAL 'Infinity' AS varchar(7))", INVALID_CAST_ARGUMENT, "Value Infinity (Infinity) cannot be represented as varchar(7)"); + assertPrestoExceptionThrownBy("CAST(REAL '12e2' AS varchar(3)) = '1200.0'", INVALID_CAST_ARGUMENT, "Value 1200.0 (1.2E3) cannot be represented as varchar(3)"); } private static void assertSimplifies(String expression, String expected) diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestRealOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestRealOperators.java index 3940213d64b24..6ce7b7caed1b9 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestRealOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestRealOperators.java @@ -171,21 +171,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