Fix precision loss in parse_duration for large millisecond values

Fixes #25340

Author: martinsander00

presto-main-base/src/main/java/com/facebook/presto/operator/scalar/DateTimeFunctions.java

@@ -38,8 +38,11 @@
 import org.joda.time.format.DateTimeFormatterBuilder;
 import org.joda.time.format.ISODateTimeFormat;
 
+import java.math.BigDecimal;
 import java.util.Locale;
 import java.util.concurrent.TimeUnit;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static com.facebook.presto.common.type.DateTimeEncoding.packDateTimeWithZone;
 import static com.facebook.presto.common.type.DateTimeEncoding.unpackMillisUtc;
@@ -83,6 +86,7 @@ public final class DateTimeFunctions
     private static final DateTimeField MONTH_OF_YEAR = UTC_CHRONOLOGY.monthOfYear();
     private static final DateTimeField QUARTER = QUARTER_OF_YEAR.getField(UTC_CHRONOLOGY);
     private static final DateTimeField YEAR = UTC_CHRONOLOGY.year();
+    private static final Pattern PATTERN = Pattern.compile("^\\s*(\\d+(?:\\.\\d+)?)\\s*([a-zA-Z]+)\\s*$");
     private static final int MILLISECONDS_IN_SECOND = 1000;
     private static final int MILLISECONDS_IN_MINUTE = 60 * MILLISECONDS_IN_SECOND;
     private static final int MILLISECONDS_IN_HOUR = 60 * MILLISECONDS_IN_MINUTE;
@@ -1437,14 +1441,53 @@ else if (character == '%') {
     @SqlType(StandardTypes.INTERVAL_DAY_TO_SECOND)
     public static long parseDuration(@SqlType("varchar(x)") Slice duration)
     {
+        String durationStr = duration.toStringUtf8();
+
+        if (durationStr.isEmpty()) {
+            throw new PrestoException(INVALID_FUNCTION_ARGUMENT, "duration is empty");
+        }
+
         try {
-            return Duration.valueOf(duration.toStringUtf8()).toMillis();
+            Matcher matcher = PATTERN.matcher(durationStr);
+
+            if (!matcher.matches()) {
+                throw new PrestoException(INVALID_FUNCTION_ARGUMENT,
+                        "duration is not a valid data duration string: " + durationStr);
+            }
+
+            BigDecimal value = new BigDecimal(matcher.group(1));
+            TimeUnit timeUnit = Duration.valueOfTimeUnit(matcher.group(2));
+
+            return value.multiply(millisPerTimeUnit(timeUnit))
+                    .add(BigDecimal.valueOf(0.5)).longValue();
         }
-        catch (IllegalArgumentException e) {
+        catch (IllegalArgumentException | ArithmeticException e) {
             throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e);
         }
     }
 
+    private static BigDecimal millisPerTimeUnit(TimeUnit timeUnit)
+    {
+        switch (timeUnit) {
+            case NANOSECONDS:
+                return new BigDecimal("0.000001");
+            case MICROSECONDS:
+                return new BigDecimal("0.001");
+            case MILLISECONDS:
+                return BigDecimal.ONE;
+            case SECONDS:
+                return BigDecimal.valueOf(1000);
+            case MINUTES:
+                return BigDecimal.valueOf(60_000);
+            case HOURS:
+                return BigDecimal.valueOf(3_600_000);
+            case DAYS:
+                return BigDecimal.valueOf(86_400_000);
+            default:
+                throw new AssertionError("Unknown TimeUnit: " + timeUnit);
+        }
+    }
+
     private static long timeAtTimeZone(SqlFunctionProperties properties, long timeWithTimeZone, TimeZoneKey timeZoneKey)
     {
         DateTimeZone sourceTimeZone = getDateTimeZone(unpackZoneKey(timeWithTimeZone));

presto-main-base/src/test/java/com/facebook/presto/operator/scalar/BenchmarkDateTimeFunctions.java

@@ -0,0 +1,94 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.operator.scalar;
+
+import io.airlift.slice.Slice;
+import io.airlift.slice.Slices;
+import io.airlift.units.Duration;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Param;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+import org.openjdk.jmh.infra.Blackhole;
+import org.openjdk.jmh.runner.Runner;
+import org.openjdk.jmh.runner.RunnerException;
+import org.openjdk.jmh.runner.options.Options;
+import org.openjdk.jmh.runner.options.OptionsBuilder;
+
+import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
+@State(Scope.Benchmark)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Fork(value = 1, jvmArgs = {"-Xms2G", "-Xmx2G"})
+@Warmup(iterations = 5, time = 1)
+@Measurement(iterations = 10, time = 1)
+@BenchmarkMode(Mode.AverageTime)
+public class BenchmarkDateTimeFunctions
+{
+    @Param({"ns", "us", "ms", "s", "m", "h", "d"})
+    private String unit = "ns";
+
+    private Random random = new Random();
+
+    @Setup
+    public void setup()
+    {
+        random = new Random(42); // Fixed seed for reproducibility
+    }
+
+    @Benchmark
+    public void testBaseline(Blackhole bh)
+    {
+        int v1 = random.nextInt(10000);
+        int v2 = random.nextInt(10000);
+        Slice value = Slices.utf8Slice(v1 + "." + v2 + " " + unit);
+        bh.consume(value.toStringUtf8());
+    }
+
+    @Benchmark
+    public void testUseBigDecimal(Blackhole bh)
+    {
+        int v1 = random.nextInt(10000);
+        int v2 = random.nextInt(10000);
+        Slice value = Slices.utf8Slice(v1 + "." + v2 + " " + unit);
+        bh.consume(DateTimeFunctions.parseDuration(value));
+    }
+
+    @Benchmark
+    public void testUseDouble(Blackhole bh)
+    {
+        int v1 = random.nextInt(10000);
+        int v2 = random.nextInt(10000);
+        Slice value = Slices.utf8Slice(v1 + "." + v2 + " " + unit);
+        bh.consume(Duration.valueOf(value.toStringUtf8()).toMillis());
+    }
+
+    public static void main(String[] args)
+            throws RunnerException
+    {
+        Options opt = new OptionsBuilder()
+                .include(BenchmarkDateTimeFunctions.class.getSimpleName())
+                .build();
+
+        new Runner(opt).run();
+    }
+}

presto-main-base/src/test/java/com/facebook/presto/operator/scalar/TestDateTimeFunctionsBase.java

@@ -1226,10 +1226,23 @@ public void testParseDuration()
         assertFunction("parse_duration('1234.567h')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(51, 10, 34, 1, 200));
         assertFunction("parse_duration('1234.567d')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(1234, 13, 36, 28, 800));
 
+        // trailing spaces
+        assertFunction("parse_duration('1234 ns ')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(0, 0, 0, 0, 0));
+        assertFunction("parse_duration('1234 us ')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(0, 0, 0, 0, 1));
+        assertFunction("parse_duration('1234ms ')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(0, 0, 0, 1, 234));
+
         // invalid function calls
         assertInvalidFunction("parse_duration('')", "duration is empty");
         assertInvalidFunction("parse_duration('1f')", "Unknown time unit: f");
         assertInvalidFunction("parse_duration('abc')", "duration is not a valid data duration string: abc");
+
+        // long milliseconds edge cases
+        assertFunction("parse_duration('7702741401940153ms')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(89152099, 13, 25, 40, 153));
+        assertFunction("parse_duration('9117756383778565ms')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(105529587, 18, 36, 18, 565));
+
+        // Test precision for large values with fractional seconds
+        assertFunction("parse_duration('7702741401940.153s')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(89152099, 13, 25, 40, 153));
+        assertFunction("parse_duration('7702741401940.153 s')", INTERVAL_DAY_TIME, new SqlIntervalDayTime(89152099, 13, 25, 40, 153));
     }
 
     @Test