Skip to content

Commit

Permalink
Fix silent clamping in double to long cast
Browse files Browse the repository at this point in the history
  • Loading branch information
Gerlou Shyy committed Jun 7, 2018
1 parent 782350d commit 2a24538
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 6 deletions.
10 changes: 9 additions & 1 deletion presto-main/src/main/java/com/facebook/presto/type/DoubleOperators.java
View file
Expand Up @@ -27,6 +27,7 @@
import io.airlift.slice.Slice;

import static com.facebook.presto.spi.StandardErrorCode.DIVISION_BY_ZERO;
import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE;
import static com.facebook.presto.spi.function.OperatorType.ADD;
import static com.facebook.presto.spi.function.OperatorType.BETWEEN;
Expand All @@ -50,7 +51,9 @@
import static java.lang.Double.doubleToLongBits;
import static java.lang.Float.floatToRawIntBits;
import static java.lang.Math.toIntExact;
import static java.lang.String.format;
import static java.math.RoundingMode.FLOOR;
import static java.math.RoundingMode.HALF_UP;

public final class DoubleOperators
{
Expand Down Expand Up @@ -217,7 +220,12 @@ public static long castToTinyint(@SqlType(StandardTypes.DOUBLE) double value)
@SqlType(StandardTypes.BIGINT)
public static long castToLong(@SqlType(StandardTypes.DOUBLE) double value)
{
return (long) MathFunctions.round(value);
try {
return DoubleMath.roundToLong(value, HALF_UP);
}
catch (ArithmeticException e) {
throw new PrestoException(INVALID_CAST_ARGUMENT, format("Unable to cast %s to bigint", value), e);
}
}

@ScalarOperator(CAST)
Expand Down
4 changes: 3 additions & 1 deletion presto-main/src/test/java/com/facebook/presto/sql/gen/TestExpressionCompiler.java
View file
Expand Up @@ -775,7 +775,9 @@ public void testCast()

for (Double value : doubleLefts) {
assertExecute(generateExpression("cast(%s as boolean)", value), BOOLEAN, value == null ? null : (value != 0.0 ? true : false));
assertExecute(generateExpression("cast(%s as bigint)", value), BIGINT, value == null ? null : value.longValue());
if (value == null || (value >= Long.MIN_VALUE && value < Long.MAX_VALUE)) {
assertExecute(generateExpression("cast(%s as bigint)", value), BIGINT, value == null ? null : value.longValue());
}
assertExecute(generateExpression("cast(%s as double)", value), DOUBLE, value == null ? null : value);
assertExecute(generateExpression("cast(%s as varchar)", value), VARCHAR, value == null ? null : String.valueOf(value));
}
Expand Down
23 changes: 23 additions & 0 deletions presto-main/src/test/java/com/facebook/presto/type/TestDoubleOperators.java
View file
Expand Up @@ -16,6 +16,7 @@
import com.facebook.presto.operator.scalar.AbstractTestFunctions;
import org.testng.annotations.Test;

import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static com.facebook.presto.spi.type.BigintType.BIGINT;
import static com.facebook.presto.spi.type.BooleanType.BOOLEAN;
import static com.facebook.presto.spi.type.DoubleType.DOUBLE;
Expand Down Expand Up @@ -176,7 +177,29 @@ public void testCastToVarchar()
public void testCastToBigint()
{
assertFunction("cast(37.7E0 as bigint)", BIGINT, 38L);
assertFunction("cast(-37.7E0 as bigint)", BIGINT, -38L);
assertFunction("cast(17.1E0 as bigint)", BIGINT, 17L);
assertFunction("cast(-17.1E0 as bigint)", BIGINT, -17L);
assertFunction("cast(9.2E18 as bigint)", BIGINT, 9200000000000000000L);
assertFunction("cast(-9.2E18 as bigint)", BIGINT, -9200000000000000000L);
assertFunction("cast(2.21E9 as bigint)", BIGINT, 2210000000L);
assertFunction("cast(-2.21E9 as bigint)", BIGINT, -2210000000L);
assertFunction("cast(17.5E0 as bigint)", BIGINT, 18L);
assertFunction("cast(-17.5E0 as bigint)", BIGINT, -18L);

assertFunction("cast(" + Math.nextDown(0x1.0p63) + " as bigint)", BIGINT, (long) Math.nextDown(0x1.0p63));
assertInvalidFunction("cast(" + 0x1.0p63 + " as bigint)", INVALID_CAST_ARGUMENT);
assertInvalidFunction("cast(" + Math.nextUp(0x1.0p63) + " as bigint)", INVALID_CAST_ARGUMENT);
assertInvalidFunction("cast(" + Math.nextDown(-0x1.0p63) + " as bigint)", INVALID_CAST_ARGUMENT);
assertFunction("cast(" + -0x1.0p63 + " as bigint)", BIGINT, (long) -0x1.0p63);
assertFunction("cast(" + Math.nextUp(-0x1.0p63) + " as bigint)", BIGINT, (long) Math.nextUp(-0x1.0p63));

assertInvalidFunction("cast(9.3E18 as bigint)", INVALID_CAST_ARGUMENT);
assertInvalidFunction("cast(-9.3E18 as bigint)", INVALID_CAST_ARGUMENT);

assertInvalidFunction("cast(infinity() as bigint)", INVALID_CAST_ARGUMENT);
assertInvalidFunction("cast(-infinity() as bigint)", INVALID_CAST_ARGUMENT);
assertInvalidFunction("cast(nan() as bigint)", INVALID_CAST_ARGUMENT);
}

@Test
Expand Down
8 changes: 4 additions & 4 deletions presto-main/src/test/java/com/facebook/presto/type/TestJsonOperators.java
View file
Expand Up @@ -77,7 +77,7 @@ public void testCastToBigint()
assertInvalidFunction("cast(JSON '12345678901234567890' as BIGINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '128.9' as BIGINT)", BIGINT, 129L);
assertFunction("cast(JSON '1234567890123456789.0' as BIGINT)", BIGINT, 1234567890123456768L); // loss of precision
assertFunction("cast(JSON '12345678901234567890.0' as BIGINT)", BIGINT, 9223372036854775807L); // overflow. unexpected behavior. coherent with rest of Presto.
assertInvalidFunction("cast(JSON '12345678901234567890.0' as BIGINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '1e-324' as BIGINT)", BIGINT, 0L);
assertInvalidFunction("cast(JSON '1e309' as BIGINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON 'true' as BIGINT)", BIGINT, 1L);
Expand All @@ -101,7 +101,7 @@ public void testCastToInteger()
assertFunction("cast(JSON '128' as INTEGER)", INTEGER, 128);
assertInvalidFunction("cast(JSON '12345678901' as INTEGER)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '128.9' as INTEGER)", INTEGER, 129);
assertInvalidFunction("cast(JSON '12345678901.0' as INTEGER)", INVALID_CAST_ARGUMENT); // overflow. unexpected behavior. coherent with rest of Presto.
assertInvalidFunction("cast(JSON '12345678901.0' as INTEGER)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '1e-324' as INTEGER)", INTEGER, 0);
assertInvalidFunction("cast(JSON '1e309' as INTEGER)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON 'true' as INTEGER)", INTEGER, 1);
Expand All @@ -125,7 +125,7 @@ public void testCastToSmallint()
assertFunction("cast(JSON '128' as SMALLINT)", SMALLINT, (short) 128);
assertInvalidFunction("cast(JSON '123456' as SMALLINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '128.9' as SMALLINT)", SMALLINT, (short) 129);
assertInvalidFunction("cast(JSON '123456.0' as SMALLINT)", INVALID_CAST_ARGUMENT); // overflow. unexpected behavior. coherent with rest of Presto.
assertInvalidFunction("cast(JSON '123456.0' as SMALLINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '1e-324' as SMALLINT)", SMALLINT, (short) 0);
assertInvalidFunction("cast(JSON '1e309' as SMALLINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON 'true' as SMALLINT)", SMALLINT, (short) 1);
Expand All @@ -149,7 +149,7 @@ public void testCastToTinyint()
assertFunction("cast(JSON '12' as TINYINT)", TINYINT, (byte) 12);
assertInvalidFunction("cast(JSON '1234' as TINYINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '12.9' as TINYINT)", TINYINT, (byte) 13);
assertInvalidFunction("cast(JSON '1234.0' as TINYINT)", INVALID_CAST_ARGUMENT); // overflow. unexpected behavior. coherent with rest of Presto.
assertInvalidFunction("cast(JSON '1234.0' as TINYINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON '1e-324' as TINYINT)", TINYINT, (byte) 0);
assertInvalidFunction("cast(JSON '1e309' as TINYINT)", INVALID_CAST_ARGUMENT);
assertFunction("cast(JSON 'true' as TINYINT)", TINYINT, (byte) 1);
Expand Down

0 comments on commit 2a24538

Please sign in to comment.