diff --git a/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java b/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java index d8a8500379cc0..bf8d8e74afedb 100644 --- a/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java +++ b/presto-main/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java @@ -182,7 +182,13 @@ public static long localTimestamp(SqlFunctionProperties properties) @SqlType(StandardTypes.TIMESTAMP) public static long fromUnixTime(@SqlType(StandardTypes.DOUBLE) double unixTime) { - return Math.round(unixTime * 1000); + // This implementation fixes previous issue of precision loss when it comes for some edge cases. + // For example, 1.7041507095805E9 would correctly yield "2024-01-01 15:11:49.580" + // Machine-representable double for the 1.7041507095805E9 is 1704150709.58049988746643066406. + // 1704150709 goes to seconds and 0.580499887466 should be rounded to 580 milliseconds. + // Previous implementation would wrongly result in 581 milliseconds. + // Reference: https://github.com/prestodb/presto/issues/21891#issue-2126580070 + return Math.round(Math.floor(unixTime) * 1000 + Math.round((unixTime - Math.floor(unixTime)) * 1000)); } @ScalarFunction("from_unixtime") diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java index 2c79a9fd3b75a..2f3be9168f9af 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java @@ -19,6 +19,7 @@ import com.facebook.presto.common.type.SqlDate; import com.facebook.presto.common.type.SqlTime; import com.facebook.presto.common.type.SqlTimeWithTimeZone; +import com.facebook.presto.common.type.SqlTimestamp; import com.facebook.presto.common.type.SqlTimestampWithTimeZone; import com.facebook.presto.common.type.TimeType; import com.facebook.presto.common.type.TimeZoneKey; @@ -78,6 +79,7 @@ import static java.util.concurrent.TimeUnit.NANOSECONDS; import static java.util.concurrent.TimeUnit.SECONDS; import static org.joda.time.DateTimeUtils.getInstantChronology; +import static org.joda.time.DateTimeZone.UTC; import static org.joda.time.Days.daysBetween; import static org.joda.time.DurationFieldType.millis; import static org.joda.time.Months.monthsBetween; @@ -193,6 +195,10 @@ public void testFromUnixTime() dateTime = new DateTime(2001, 1, 22, 3, 4, 5, 888, DATE_TIME_ZONE); seconds = dateTime.getMillis() / 1000.0; assertFunction("from_unixtime(" + seconds + ")", TimestampType.TIMESTAMP, sqlTimestampOf(dateTime, session)); + + // The particular double, 1.7041507095805E9, was causing loss of precision in the function before the fix #21899 + SqlTimestamp expected = sqlTimestampOf(2024, 1, 1, 23, 11, 49, 580, UTC, session.getTimeZoneKey(), session.toConnectorSession()); + assertFunction("from_unixtime(1.7041507095805E9)", TimestampType.TIMESTAMP, expected); } @Test