From 1dcb9f84cef14cca40d3e3851cc42024ce485d8a Mon Sep 17 00:00:00 2001 From: Asjad Syed Date: Wed, 23 Mar 2022 17:43:40 -0400 Subject: [PATCH 1/5] Revert "Fix cast from real to varchar: follow the SQL standard in formatting" This reverts commit 65ecdd6553c1cf476a6b9b8bc0ffd50678cdfdcd. --- .../facebook/presto/type/RealOperators.java | 34 +------- .../presto/sql/TestExpressionInterpreter.java | 78 +++++++++---------- .../rule/TestSimplifyExpressions.java | 49 +++++------- .../presto/type/TestRealOperators.java | 22 +++--- 4 files changed, 71 insertions(+), 112 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 4f43482ff6380..dc4447be5cb4d 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,8 +31,6 @@ 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; @@ -74,8 +72,6 @@ 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() { } @@ -193,39 +189,13 @@ 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) { - 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 stringValue = String.valueOf(intBitsToFloat((int) value)); // 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 (%s) cannot be represented as varchar(%s)", floatValue, stringValue, x)); + throw new PrestoException(INVALID_CAST_ARGUMENT, format("Value %s cannot be represented as varchar(%s)", 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 eeb666678c28f..1c45fea686853 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,21 +588,18 @@ 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; } @@ -610,13 +607,11 @@ 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; } @@ -680,37 +675,38 @@ 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))", "'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'"); + 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)"); } @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 b486586c9a60b..1680c5b253555 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,8 +138,7 @@ 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. @@ -149,13 +148,11 @@ 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; } @@ -163,13 +160,11 @@ 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; } @@ -185,35 +180,33 @@ 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'"); - - // 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'"); + assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1.2E3' AS varchar(50))"); /// cast from double to varchar fails - 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)"); + 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)"); } @Test public void testCastRealToBoundedVarchar() { // the varchar type length is enough to contain the number's representation - assertSimplifies("CAST(REAL '0e0' AS varchar(3))", "'0E0'"); - assertSimplifies("CAST(REAL '-0e0' AS varchar(4))", "'-0E0'"); + assertSimplifies("CAST(REAL '0e0' AS varchar(3))", "'0.0'"); + assertSimplifies("CAST(REAL '-0e0' AS varchar(4))", "'-0.0'"); assertSimplifies("CAST(REAL '0e0' / REAL '0e0' AS varchar(3))", "'NaN'"); assertSimplifies("CAST(REAL 'Infinity' AS varchar(8))", "'Infinity'"); - assertSimplifies("CAST(REAL '12e2' AS varchar(5))", "'1.2E3'"); - //assertSimplifies("CAST(REAL '-12e2' AS varchar(50))", "CAST('-1.2E3' AS varchar(50))"); + assertSimplifies("CAST(REAL '12e2' AS varchar(6))", "'1200.0'"); + assertSimplifies("CAST(REAL '-12e2' AS varchar(50))", "CAST('-1200.0' AS varchar(50))"); - // 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)"); + // 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'"); } 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 6ce7b7caed1b9..3940213d64b24 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, "7.541985E2"); - assertFunction("CAST(REAL '-754.2008' as VARCHAR)", VARCHAR, "-7.542008E2"); + assertFunction("CAST(REAL'754.1985' as VARCHAR)", VARCHAR, "754.1985"); + assertFunction("CAST(REAL'-754.2008' as VARCHAR)", VARCHAR, "-754.2008"); 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), "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 '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 'NaN' as varchar(3))", createVarcharType(3), "NaN"); assertFunction("cast(REAL 'Infinity' as varchar(50))", createVarcharType(50), "Infinity"); - 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)"); + 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)"); } @Test From 09420673ce442911b8c1b660d574138735d21994 Mon Sep 17 00:00:00 2001 From: Asjad Syed Date: Wed, 23 Mar 2022 17:43:53 -0400 Subject: [PATCH 2/5] Revert "Fix cast from double to varchar: follow the SQL standard in formatting" This reverts commit 62b0d7b308e3b7b698389c5c6050c02301696b3a. --- .../facebook/presto/type/DoubleOperators.java | 33 +---------- .../scalar/TestArrayTransformFunction.java | 2 +- .../operator/scalar/TestLambdaExpression.java | 2 +- .../scalar/TestMapTransformKeyFunction.java | 4 +- .../scalar/TestMapTransformValueFunction.java | 4 +- .../presto/sql/TestExpressionInterpreter.java | 57 ++++++++++--------- .../sql/gen/TestExpressionCompiler.java | 3 +- .../rule/TestSimplifyExpressions.java | 8 +-- .../presto/type/TestArrayOperators.java | 4 +- .../presto/type/TestDoubleOperators.java | 20 +++---- .../presto/type/TestJsonOperators.java | 4 +- .../presto/type/TestMapOperators.java | 2 +- 12 files changed, 57 insertions(+), 86 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java b/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java index ac0806f06ebea..b87b2c6faf261 100644 --- a/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java +++ b/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java @@ -31,8 +31,6 @@ 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; @@ -76,8 +74,6 @@ 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() { } @@ -254,38 +250,13 @@ 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; - - // 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 if (Double.isNaN(value)) { - stringValue = "NaN"; - } - else { - stringValue = FORMAT.get().format(value); - } - + String stringValue = String.valueOf(value); // 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 (%s) cannot be represented as varchar(%s)", value, stringValue, x)); + throw new PrestoException(INVALID_CAST_ARGUMENT, format("Value %s cannot be represented as varchar(%s)", value, x)); } @ScalarOperator(HASH_CODE) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestArrayTransformFunction.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestArrayTransformFunction.java index 7eecf9a5dfdb2..863cd985a0ec8 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestArrayTransformFunction.java +++ b/presto-main/src/test/java/com/facebook/presto/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("2.56E1", "2.73E1")); + 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 -> MAP(ARRAY[x + 1], ARRAY[true]))", new ArrayType(mapType(DOUBLE, BOOLEAN)), diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestLambdaExpression.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestLambdaExpression.java index f96db64e9ff04..8e96cf411b958 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestLambdaExpression.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestLambdaExpression.java @@ -140,7 +140,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(), "2.56E1"); + assertFunction("apply(25.6E0, x -> CAST(x AS VARCHAR))", createUnboundedVarcharType(), "25.6"); 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/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformKeyFunction.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformKeyFunction.java index 8b08d16f12082..ba0c4d153575f 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformKeyFunction.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformKeyFunction.java @@ -152,11 +152,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("2.55E1a", "abc", "2.65E1d", "def", "2.75E1x", "xyz")); + ImmutableMap.of("25.5a", "abc", "26.5d", "def", "27.5x", "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("2.55E1", "a"), ImmutableList.of("a"), ImmutableList.of("2.65E1", "b"), ImmutableList.of("b"))); + ImmutableMap.of(ImmutableList.of("25.5", "a"), ImmutableList.of("a"), ImmutableList.of("26.5", "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/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformValueFunction.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformValueFunction.java index e04bfa6539619..025888c761768 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformValueFunction.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestMapTransformValueFunction.java @@ -101,7 +101,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.0E0", 2, "two_1.4E0", 3, "three_1.7E0")); + ImmutableMap.of(1, "one_1.0", 2, "two_1.4", 3, "three_1.7")); } @Test @@ -177,7 +177,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:2.55E1", "s1", "s1:2.65E1", "s2", "s2:2.75E1")); + ImmutableMap.of("s0", "s0:25.5", "s1", "s1:26.5", "s2", "s2:27.5")); 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/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..67bb837426482 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 @@ -566,10 +566,10 @@ public void testCastToString() assertOptimizedEquals("cast(-12300000000 as VARCHAR)", "'-12300000000'"); // double - 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'"); + 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'"); // boolean assertOptimizedEquals("cast(true as VARCHAR)", "'true'"); @@ -629,38 +629,39 @@ public void testCastDoubleToBoundedVarchar() assertEvaluatedEquals("CAST(DOUBLE 'Infinity' AS varchar(8))", "'Infinity'"); assertEvaluatedEquals("CAST(DOUBLE 'Infinity' AS varchar(50))", "'Infinity'"); - 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'"); + // 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(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(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(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(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(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(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(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(50))", "'-12.0'"); + assertEvaluatedEquals("CAST(-12e2 AS varchar(50))", "'-1200.0'"); + assertEvaluatedEquals("CAST(-12e-2 AS varchar(50))", "'-0.12'"); + // 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 - assertPrestoExceptionThrownBy("CAST(REAL '12' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 12.0 (1.2E1) cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '-12e2' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value -1200.0 (-1.2E3) cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '0' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 0.0 (0E0) cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '0e0' / REAL '0e0' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value NaN (NaN) cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL 'Infinity' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value Infinity (Infinity) cannot be represented as varchar(1)"); - - assertEvaluatedEquals("CAST(1200000e0 AS varchar(5))", "'1.2E6'"); + // the result value does not fit in the type (also, it is not compliant with the SQL standard) + assertPrestoExceptionThrownBy("CAST(REAL '12' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 12.0 cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(REAL '-12e2' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value -1200.0 cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(REAL '0' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 0.0 cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(REAL '0e0' / REAL '0e0' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value NaN cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(REAL 'Infinity' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value Infinity cannot be represented as varchar(1)"); + assertPrestoExceptionThrownBy("CAST(REAL '1200000' AS varchar(5))", INVALID_CAST_ARGUMENT, "Value 1200000.0 cannot be represented as varchar(5)"); } @Test diff --git a/presto-main/src/test/java/com/facebook/presto/sql/gen/TestExpressionCompiler.java b/presto-main/src/test/java/com/facebook/presto/sql/gen/TestExpressionCompiler.java index 05a4814f4ad05..13085507f3829 100644 --- a/presto-main/src/test/java/com/facebook/presto/sql/gen/TestExpressionCompiler.java +++ b/presto-main/src/test/java/com/facebook/presto/sql/gen/TestExpressionCompiler.java @@ -805,14 +805,13 @@ 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 : doubleFormat.format(value)); + assertExecute(generateExpression("cast(%s as varchar)", value), VARCHAR, value == null ? null : String.valueOf(value)); } assertExecute("cast('true' as boolean)", BOOLEAN, true); 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..320cd54fbcd6e 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 @@ -175,12 +175,12 @@ public void testCastBigintToBoundedVarchar() { public void testCastDoubleToBoundedVarchar() { // the varchar type length is enough to contain the number's representation - assertSimplifies("CAST(0e0 AS varchar(3))", "'0E0'"); - assertSimplifies("CAST(-0e0 AS varchar(4))", "'-0E0'"); + assertSimplifies("CAST(0e0 AS varchar(3))", "'0.0'"); + assertSimplifies("CAST(-0e0 AS varchar(4))", "'-0.0'"); 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))"); + assertSimplifies("CAST(12e2 AS varchar(6))", "'1200.0'"); + //assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1200.0' AS varchar(50))"); /// cast from double to varchar fails assertPrestoExceptionThrownBy("CAST(12e2 AS varchar(3))", INVALID_CAST_ARGUMENT, "Value 1200.0 cannot be represented as varchar(3)"); diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java index 86355c14a8a51..853bf43650bc9 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java @@ -282,7 +282,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", "1.23E1", "puppies", "kittens", "null", "", null)); + asList("true", "false", "12", "12.3", "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]")); @@ -571,7 +571,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.0E0x2.1E0x3.3E0"); + assertFunction("ARRAY_JOIN(ARRAY [1.0, DOUBLE '002.100', 3.3], 'x')", VARCHAR, "1.0x2.1x3.3"); assertInvalidFunction("ARRAY_JOIN(ARRAY [ARRAY [1], ARRAY [2]], '-')", INVALID_FUNCTION_ARGUMENT); assertInvalidFunction("ARRAY_JOIN(ARRAY [MAP(ARRAY [1], ARRAY [2])], '-')", INVALID_FUNCTION_ARGUMENT); diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java index 4121845615caa..f2b4e5e9c615c 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java @@ -177,19 +177,19 @@ public void testBetween() @Test public void testCastToVarchar() { - assertFunction("cast(37.7E0 as varchar)", VARCHAR, "3.77E1"); - assertFunction("cast(17.1E0 as varchar)", VARCHAR, "1.71E1"); - assertFunction("cast(12e2 as varchar(6))", createVarcharType(6), "1.2E3"); - assertFunction("cast(12e2 as varchar(50))", createVarcharType(50), "1.2E3"); + assertFunction("cast(37.7E0 as varchar)", VARCHAR, "37.7"); + assertFunction("cast(17.1E0 as varchar)", VARCHAR, "17.1"); + assertFunction("cast(12e2 as varchar(6))", createVarcharType(6), "1200.0"); + assertFunction("cast(12e2 as varchar(50))", createVarcharType(50), "1200.0"); 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"); - 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)"); + 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)"); } @Test diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestJsonOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestJsonOperators.java index 5c85d1ce7e35d..c325ce345f4a4 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestJsonOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestJsonOperators.java @@ -326,8 +326,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, "1.289E2"); - assertFunction("cast(JSON '1e-324' as VARCHAR)", VARCHAR, "0E0"); // smaller than minimum subnormal positive + 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 '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/presto-main/src/test/java/com/facebook/presto/type/TestMapOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestMapOperators.java index 88ee884b199dd..b285ed75fd0a3 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestMapOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestMapOperators.java @@ -375,7 +375,7 @@ public void testJsonToMap() mapType(BIGINT, VARCHAR), asMap( ImmutableList.of(1L, 2L, 3L, 5L, 8L, 13L, 21L, 34L, 55L), - asList("true", "false", "12", "1.23E1", "puppies", "kittens", "null", "", null))); + asList("true", "false", "12", "12.3", "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 8398a243f6397b4a88b46b9e4a9f4e5931ce4b5e Mon Sep 17 00:00:00 2001 From: Asjad Syed Date: Wed, 23 Mar 2022 17:44:01 -0400 Subject: [PATCH 3/5] Revert "Fix cast from real to varchar: do not return values overflowing the type" This reverts commit 19e5853137efac6ea9e91eba9fe068017164de72. --- .../facebook/presto/type/RealOperators.java | 12 ++---------- .../presto/sql/TestExpressionInterpreter.java | 14 +++++++------- .../rule/TestSimplifyExpressions.java | 18 ++++++++++++------ .../presto/type/TestRealOperators.java | 12 ++++++------ 4 files changed, 27 insertions(+), 29 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..cea219b065b90 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 @@ -51,14 +51,12 @@ import static com.facebook.presto.common.function.OperatorType.SUBTRACT; import static com.facebook.presto.common.function.OperatorType.XX_HASH_64; import static com.facebook.presto.common.type.RealType.REAL; -import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; import static io.airlift.slice.Slices.utf8Slice; import static java.lang.Float.floatToIntBits; import static java.lang.Float.floatToRawIntBits; import static java.lang.Float.intBitsToFloat; import static java.lang.Math.toIntExact; -import static java.lang.String.format; import static java.math.RoundingMode.FLOOR; public final class RealOperators @@ -187,15 +185,9 @@ public static long xxHash64(@SqlType(StandardTypes.REAL) long value) @ScalarOperator(CAST) @LiteralParameters("x") @SqlType("varchar(x)") - public static Slice castToVarchar(@LiteralParameter("x") long x, @SqlType(StandardTypes.REAL) long value) + public static Slice castToVarchar(@SqlType(StandardTypes.REAL) long value) { - String stringValue = String.valueOf(intBitsToFloat((int) value)); - // 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)); + return utf8Slice(String.valueOf(intBitsToFloat((int) value))); } @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 67bb837426482..3eadac7762c8a 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 @@ -655,13 +655,13 @@ public void testCastDoubleToBoundedVarchar() 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) - assertPrestoExceptionThrownBy("CAST(REAL '12' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 12.0 cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '-12e2' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value -1200.0 cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '0' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value 0.0 cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '0e0' / REAL '0e0' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value NaN cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL 'Infinity' AS varchar(1))", INVALID_CAST_ARGUMENT, "Value Infinity cannot be represented as varchar(1)"); - assertPrestoExceptionThrownBy("CAST(REAL '1200000' AS varchar(5))", INVALID_CAST_ARGUMENT, "Value 1200000.0 cannot be represented as varchar(5)"); + // incorrect behavior: the result value does not fit in the type (also, it is not compliant with the SQL standard) + assertEvaluatedEquals("CAST(12e0 AS varchar(1))", "'12.0'"); + assertEvaluatedEquals("CAST(-12e2 AS varchar(1))", "'-1200.0'"); + assertEvaluatedEquals("CAST(0e0 AS varchar(1))", "'0.0'"); + assertEvaluatedEquals("CAST(0e0 / 0e0 AS varchar(1))", "'NaN'"); + assertEvaluatedEquals("CAST(DOUBLE 'Infinity' AS varchar(1))", "'Infinity'"); + assertEvaluatedEquals("CAST(1200000e0 AS varchar(5))", "'1200000.0'"); } @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 320cd54fbcd6e..733eae0aeaece 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 @@ -201,12 +201,18 @@ public void testCastRealToBoundedVarchar() assertSimplifies("CAST(REAL '12e2' AS varchar(6))", "'1200.0'"); assertSimplifies("CAST(REAL '-12e2' AS varchar(50))", "CAST('-1200.0' 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'"); + // the varchar type length is not enough to contain the number's representation: + // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) + // the value is then wrapped in another cast by the LiteralEncoder (CAST('1200.0' AS varchar(3))), + // so eventually we get a truncated string '120' + assertSimplifies("CAST(REAL '12e2' AS varchar(3))", "CAST('1200.0' AS varchar(3))"); + assertSimplifies("CAST(REAL '-12e2' AS varchar(3))", "CAST('-1200.0' AS varchar(3))"); + assertSimplifies("CAST(REAL 'NaN' AS varchar(2))", "CAST('NaN' AS varchar(2))"); + assertSimplifies("CAST(REAL 'Infinity' AS varchar(7))", "CAST('Infinity' AS varchar(7))"); + + // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) + // the value is nested in a comparison expression, so it is not truncated by the LiteralEncoder + assertSimplifies("CAST(REAL '12e2' AS varchar(3)) = '1200.0'", "true"); } 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..bf9a2d4d2c865 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 @@ -180,12 +180,12 @@ public void testCastToVarchar() assertFunction("cast(REAL '12345678.9e0' as varchar(50))", createVarcharType(50), "1.2345679E7"); 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)"); + assertFunctionThrowsIncorrectly("cast(REAL '12e2' as varchar(5))", IllegalArgumentException.class, "Character count exceeds length limit 5.*"); + assertFunctionThrowsIncorrectly("cast(REAL '12e2' as varchar(4))", IllegalArgumentException.class, "Character count exceeds length limit 4.*"); + assertFunctionThrowsIncorrectly("cast(REAL '0e0' as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); + assertFunctionThrowsIncorrectly("cast(REAL '-0e0' as varchar(3))", IllegalArgumentException.class, "Character count exceeds length limit 3.*"); + assertFunctionThrowsIncorrectly("cast(REAL '0e0' / REAL '0e0' as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); + assertFunctionThrowsIncorrectly("cast(REAL 'Infinity' as varchar(7))", IllegalArgumentException.class, "Character count exceeds length limit 7.*"); } @Test From fdf116cb34cb1b15adf784094075130f2c9eb2b5 Mon Sep 17 00:00:00 2001 From: Asjad Syed Date: Wed, 23 Mar 2022 17:44:13 -0400 Subject: [PATCH 4/5] Revert "Fix cast from double to varchar" This reverts commit a8d85317f5c3cb5efb7d1ffc93974d74630a9148. --- .../facebook/presto/type/DoubleOperators.java | 10 ++---- .../com/facebook/presto/util/JsonUtil.java | 3 +- .../presto/sql/TestExpressionInterpreter.java | 34 ++++--------------- .../rule/TestSimplifyExpressions.java | 19 +++++++---- .../presto/type/TestDoubleOperators.java | 12 +++---- 5 files changed, 28 insertions(+), 50 deletions(-) diff --git a/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java b/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java index b87b2c6faf261..699c5d93e3a25 100644 --- a/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java +++ b/presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java @@ -248,15 +248,9 @@ public static long castToReal(@SqlType(StandardTypes.DOUBLE) double value) @ScalarOperator(CAST) @LiteralParameters("x") @SqlType("varchar(x)") - public static Slice castToVarchar(@LiteralParameter("x") long x, @SqlType(StandardTypes.DOUBLE) double value) + public static Slice castToVarchar(@SqlType(StandardTypes.DOUBLE) double value) { - String stringValue = String.valueOf(value); - // 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)", value, x)); + return utf8Slice(String.valueOf(value)); } @ScalarOperator(HASH_CODE) diff --git a/presto-main/src/main/java/com/facebook/presto/util/JsonUtil.java b/presto-main/src/main/java/com/facebook/presto/util/JsonUtil.java index 7c824f8d0bba0..79e014a29cbb1 100644 --- a/presto-main/src/main/java/com/facebook/presto/util/JsonUtil.java +++ b/presto-main/src/main/java/com/facebook/presto/util/JsonUtil.java @@ -56,7 +56,6 @@ import java.util.Optional; import java.util.TreeMap; -import static com.facebook.presto.common.type.AbstractVarcharType.UNBOUNDED_LENGTH; import static com.facebook.presto.common.type.BigintType.BIGINT; import static com.facebook.presto.common.type.BooleanType.BOOLEAN; import static com.facebook.presto.common.type.DateType.DATE; @@ -652,7 +651,7 @@ public static Slice currentTokenAsVarchar(JsonParser parser) return Slices.utf8Slice(parser.getText()); case VALUE_NUMBER_FLOAT: // Avoidance of loss of precision does not seem to be possible here because of Jackson implementation. - return DoubleOperators.castToVarchar(UNBOUNDED_LENGTH, parser.getDoubleValue()); + return DoubleOperators.castToVarchar(parser.getDoubleValue()); case VALUE_NUMBER_INT: // An alternative is calling getLongValue and then BigintOperators.castToVarchar. // It doesn't work as well because it can result in overflow and underflow exceptions for large integral numbers. 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 3eadac7762c8a..1657b9667e4f2 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 @@ -26,7 +26,6 @@ import com.facebook.presto.metadata.MetadataManager; import com.facebook.presto.operator.scalar.FunctionAssertions; import com.facebook.presto.spi.PrestoException; -import com.facebook.presto.spi.StandardErrorCode; import com.facebook.presto.spi.WarningCollector; import com.facebook.presto.spi.relation.CallExpression; import com.facebook.presto.spi.relation.ConstantExpression; @@ -700,14 +699,13 @@ public void testCastRealToBoundedVarchar() 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)"); + // incorrect behavior: the result value does not fit in the type (also, it is not compliant with the SQL standard) + assertEvaluatedEquals("CAST(REAL '12' AS varchar(1))", "'12.0'"); + assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(1))", "'-1200.0'"); + assertEvaluatedEquals("CAST(REAL '0' AS varchar(1))", "'0.0'"); + assertEvaluatedEquals("CAST(REAL '0e0' / REAL '0e0' AS varchar(1))", "'NaN'"); + assertEvaluatedEquals("CAST(REAL 'Infinity' AS varchar(1))", "'Infinity'"); + assertEvaluatedEquals("CAST(REAL '1200000' AS varchar(5))", "'1200000.0'"); } @Test @@ -1887,24 +1885,6 @@ private static Object evaluate(Expression expression, boolean deterministic) return expressionResult; } - public static void assertPrestoExceptionThrownBy(String expression, StandardErrorCode errorCode, String message) - { - try { - evaluate(expression, true); - fail(format("Expected to throw exception %s", errorCode.toString())); - } - catch (PrestoException e) { - try { - assertEquals(e.getErrorCode(), errorCode.toErrorCode()); - assertEquals(e.getMessage(), message); - } - catch (Throwable failure) { - failure.addSuppressed(e); - throw failure; - } - } - } - private static class FailedFunctionRewriter extends ExpressionRewriter { 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 733eae0aeaece..21fa67f42c96d 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 @@ -48,7 +48,6 @@ import static com.facebook.presto.sql.ExpressionUtils.binaryExpression; import static com.facebook.presto.sql.ExpressionUtils.extractPredicates; import static com.facebook.presto.sql.ExpressionUtils.rewriteIdentifiersToSymbolReferences; -import static com.facebook.presto.sql.TestExpressionInterpreter.assertPrestoExceptionThrownBy; import static com.facebook.presto.sql.planner.iterative.rule.SimplifyExpressions.rewrite; import static com.google.common.collect.ImmutableSet.toImmutableSet; import static java.lang.String.format; @@ -182,12 +181,18 @@ public void testCastDoubleToBoundedVarchar() assertSimplifies("CAST(12e2 AS varchar(6))", "'1200.0'"); //assertSimplifies("CAST(-12e2 AS varchar(50))", "CAST('-1200.0' AS varchar(50))"); - /// 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)"); + // the varchar type length is not enough to contain the number's representation: + // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) + // the value is then wrapped in another cast by the LiteralEncoder (CAST('1200.0' AS varchar(3))), + // so eventually we get a truncated string '120' + assertSimplifies("CAST(12e2 AS varchar(3))", "CAST('1200.0' AS varchar(3))"); + assertSimplifies("CAST(-12e2 AS varchar(3))", "CAST('-1200.0' AS varchar(3))"); + assertSimplifies("CAST(DOUBLE 'NaN' AS varchar(2))", "CAST('NaN' AS varchar(2))"); + assertSimplifies("CAST(DOUBLE 'Infinity' AS varchar(7))", "CAST('Infinity' AS varchar(7))"); + + // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) + // the value is nested in a comparison expression, so it is not truncated by the LiteralEncoder + assertSimplifies("CAST(12e2 AS varchar(3)) = '1200.0'", "true"); } @Test diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java index f2b4e5e9c615c..716f4c139dcb2 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java @@ -184,12 +184,12 @@ public void testCastToVarchar() 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)"); + assertFunctionThrowsIncorrectly("cast(12e2 as varchar(5))", IllegalArgumentException.class, "Character count exceeds length limit 5.*"); + assertFunctionThrowsIncorrectly("cast(12e2 as varchar(4))", IllegalArgumentException.class, "Character count exceeds length limit 4.*"); + assertFunctionThrowsIncorrectly("cast(0e0 as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); + assertFunctionThrowsIncorrectly("cast(-0e0 as varchar(3))", IllegalArgumentException.class, "Character count exceeds length limit 3.*"); + assertFunctionThrowsIncorrectly("cast(0e0 / 0e0 as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); + assertFunctionThrowsIncorrectly("cast(DOUBLE 'Infinity' as varchar(7))", IllegalArgumentException.class, "Character count exceeds length limit 7.*"); } @Test From 4ad362e1cef4fa571b90230e266c8378dee02918 Mon Sep 17 00:00:00 2001 From: Asjad Syed Date: Wed, 23 Mar 2022 17:44:20 -0400 Subject: [PATCH 5/5] Revert "Test casts of approximate numeric types to bounded varchar" This reverts commit 0ae645b1bb8af1babc3e525552eea6d04496e221. --- .../presto/sql/TestExpressionInterpreter.java | 106 ++---------------- .../rule/TestSimplifyExpressions.java | 65 ++--------- .../presto/type/TestDoubleOperators.java | 12 -- .../presto/type/TestRealOperators.java | 12 -- 4 files changed, 20 insertions(+), 175 deletions(-) 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 1657b9667e4f2..54e5be70d79d1 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 @@ -587,18 +587,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; } @@ -606,108 +609,19 @@ 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; } } } - @Test - public void testCastDoubleToBoundedVarchar() - { - // NaN - assertEvaluatedEquals("CAST(0e0 / 0e0 AS varchar(3))", "'NaN'"); - assertEvaluatedEquals("CAST(0e0 / 0e0 AS varchar(50))", "'NaN'"); - - // Infinity - 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(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(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(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(50))", "'-12.0'"); - assertEvaluatedEquals("CAST(-12e2 AS varchar(50))", "'-1200.0'"); - assertEvaluatedEquals("CAST(-12e-2 AS varchar(50))", "'-0.12'"); - - // 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'"); - - // incorrect behavior: the result value does not fit in the type (also, it is not compliant with the SQL standard) - assertEvaluatedEquals("CAST(12e0 AS varchar(1))", "'12.0'"); - assertEvaluatedEquals("CAST(-12e2 AS varchar(1))", "'-1200.0'"); - assertEvaluatedEquals("CAST(0e0 AS varchar(1))", "'0.0'"); - assertEvaluatedEquals("CAST(0e0 / 0e0 AS varchar(1))", "'NaN'"); - assertEvaluatedEquals("CAST(DOUBLE 'Infinity' AS varchar(1))", "'Infinity'"); - assertEvaluatedEquals("CAST(1200000e0 AS varchar(5))", "'1200000.0'"); - } - - @Test - public void testCastRealToBoundedVarchar() - { - // NaN - assertEvaluatedEquals("CAST(REAL '0e0' / REAL '0e0' AS varchar(3))", "'NaN'"); - assertEvaluatedEquals("CAST(REAL '0e0' / REAL '0e0' AS varchar(50))", "'NaN'"); - - // Infinity - 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 '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'"); - - // incorrect behavior: the result value does not fit in the type (also, it is not compliant with the SQL standard) - assertEvaluatedEquals("CAST(REAL '12' AS varchar(1))", "'12.0'"); - assertEvaluatedEquals("CAST(REAL '-12e2' AS varchar(1))", "'-1200.0'"); - assertEvaluatedEquals("CAST(REAL '0' AS varchar(1))", "'0.0'"); - assertEvaluatedEquals("CAST(REAL '0e0' / REAL '0e0' AS varchar(1))", "'NaN'"); - assertEvaluatedEquals("CAST(REAL 'Infinity' AS varchar(1))", "'Infinity'"); - assertEvaluatedEquals("CAST(REAL '1200000' AS varchar(5))", "'1200000.0'"); - } - @Test public void testCastToBoolean() { 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 21fa67f42c96d..9d1453c577ba5 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 @@ -137,7 +137,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. @@ -147,11 +148,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; } @@ -159,67 +162,19 @@ 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; } } } - @Test - 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 / 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))"); - - // the varchar type length is not enough to contain the number's representation: - // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) - // the value is then wrapped in another cast by the LiteralEncoder (CAST('1200.0' AS varchar(3))), - // so eventually we get a truncated string '120' - assertSimplifies("CAST(12e2 AS varchar(3))", "CAST('1200.0' AS varchar(3))"); - assertSimplifies("CAST(-12e2 AS varchar(3))", "CAST('-1200.0' AS varchar(3))"); - assertSimplifies("CAST(DOUBLE 'NaN' AS varchar(2))", "CAST('NaN' AS varchar(2))"); - assertSimplifies("CAST(DOUBLE 'Infinity' AS varchar(7))", "CAST('Infinity' AS varchar(7))"); - - // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) - // the value is nested in a comparison expression, so it is not truncated by the LiteralEncoder - assertSimplifies("CAST(12e2 AS varchar(3)) = '1200.0'", "true"); - } - - @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' / 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))"); - - // the varchar type length is not enough to contain the number's representation: - // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) - // the value is then wrapped in another cast by the LiteralEncoder (CAST('1200.0' AS varchar(3))), - // so eventually we get a truncated string '120' - assertSimplifies("CAST(REAL '12e2' AS varchar(3))", "CAST('1200.0' AS varchar(3))"); - assertSimplifies("CAST(REAL '-12e2' AS varchar(3))", "CAST('-1200.0' AS varchar(3))"); - assertSimplifies("CAST(REAL 'NaN' AS varchar(2))", "CAST('NaN' AS varchar(2))"); - assertSimplifies("CAST(REAL 'Infinity' AS varchar(7))", "CAST('Infinity' AS varchar(7))"); - - // the cast operator returns a value that is too long for the expected type ('1200.0' for varchar(3)) - // the value is nested in a comparison expression, so it is not truncated by the LiteralEncoder - assertSimplifies("CAST(REAL '12e2' AS varchar(3)) = '1200.0'", "true"); - } - private static void assertSimplifies(String expression, String expected) { assertSimplifies(expression, expected, null); diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java index 716f4c139dcb2..32158e2ebefff 100644 --- a/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java +++ b/presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java @@ -22,7 +22,6 @@ import static com.facebook.presto.common.type.DoubleType.DOUBLE; import static com.facebook.presto.common.type.RealType.REAL; import static com.facebook.presto.common.type.VarcharType.VARCHAR; -import static com.facebook.presto.common.type.VarcharType.createVarcharType; import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; import static java.lang.Double.doubleToLongBits; import static java.lang.Double.doubleToRawLongBits; @@ -179,17 +178,6 @@ public void testCastToVarchar() { assertFunction("cast(37.7E0 as varchar)", VARCHAR, "37.7"); assertFunction("cast(17.1E0 as varchar)", VARCHAR, "17.1"); - assertFunction("cast(12e2 as varchar(6))", createVarcharType(6), "1200.0"); - assertFunction("cast(12e2 as varchar(50))", createVarcharType(50), "1200.0"); - 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"); - assertFunctionThrowsIncorrectly("cast(12e2 as varchar(5))", IllegalArgumentException.class, "Character count exceeds length limit 5.*"); - assertFunctionThrowsIncorrectly("cast(12e2 as varchar(4))", IllegalArgumentException.class, "Character count exceeds length limit 4.*"); - assertFunctionThrowsIncorrectly("cast(0e0 as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); - assertFunctionThrowsIncorrectly("cast(-0e0 as varchar(3))", IllegalArgumentException.class, "Character count exceeds length limit 3.*"); - assertFunctionThrowsIncorrectly("cast(0e0 / 0e0 as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); - assertFunctionThrowsIncorrectly("cast(DOUBLE 'Infinity' as varchar(7))", IllegalArgumentException.class, "Character count exceeds length limit 7.*"); } @Test 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 bf9a2d4d2c865..353245350fa00 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 @@ -25,7 +25,6 @@ import static com.facebook.presto.common.type.SmallintType.SMALLINT; import static com.facebook.presto.common.type.TinyintType.TINYINT; import static com.facebook.presto.common.type.VarcharType.VARCHAR; -import static com.facebook.presto.common.type.VarcharType.createVarcharType; import static java.lang.Float.floatToIntBits; import static java.lang.Float.intBitsToFloat; import static java.lang.Float.isNaN; @@ -175,17 +174,6 @@ public void testCastToVarchar() assertFunction("CAST(REAL'-754.2008' as VARCHAR)", VARCHAR, "-754.2008"); 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 'NaN' as varchar(3))", createVarcharType(3), "NaN"); - assertFunction("cast(REAL 'Infinity' as varchar(50))", createVarcharType(50), "Infinity"); - assertFunctionThrowsIncorrectly("cast(REAL '12e2' as varchar(5))", IllegalArgumentException.class, "Character count exceeds length limit 5.*"); - assertFunctionThrowsIncorrectly("cast(REAL '12e2' as varchar(4))", IllegalArgumentException.class, "Character count exceeds length limit 4.*"); - assertFunctionThrowsIncorrectly("cast(REAL '0e0' as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); - assertFunctionThrowsIncorrectly("cast(REAL '-0e0' as varchar(3))", IllegalArgumentException.class, "Character count exceeds length limit 3.*"); - assertFunctionThrowsIncorrectly("cast(REAL '0e0' / REAL '0e0' as varchar(2))", IllegalArgumentException.class, "Character count exceeds length limit 2.*"); - assertFunctionThrowsIncorrectly("cast(REAL 'Infinity' as varchar(7))", IllegalArgumentException.class, "Character count exceeds length limit 7.*"); } @Test