Reintroduce negative epoch_millis #1991 (#2232)

* Reintroduce negative epoch_millis #1991

Fixes a regression introduced with Elasticsearch 7 regarding the date
field type that removed support for negative timestamps with sub-second
granularity.

Thanks to Ryan Kophs (https://github.com/rkophs) for allowing me to use
his previous work.

Signed-off-by: Breno Faria <breno.faria@intrafind.de>

* applying spotless fix

Signed-off-by: Breno Faria <breno.faria@intrafind.de>

* more conservative implementation of isSupportedBy

Signed-off-by: Breno Faria <breno.faria@intrafind.de>

* adding braces to control flow statement

Signed-off-by: Breno Faria <breno.faria@intrafind.de>

* spotless fix...

Signed-off-by: Breno Faria <breno.faria@intrafind.de>

Co-authored-by: Breno Faria <breno.faria@intrafind.de>

server/src/main/java/org/opensearch/common/time/EpochTime.java

@@ -43,8 +43,10 @@ import java.time.temporal.TemporalAccessor;
 import java.time.temporal.TemporalField;
 import java.time.temporal.TemporalUnit;
 import java.time.temporal.ValueRange;
+import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Optional;
 
 /**
  * This class provides {@link DateTimeFormatter}s capable of parsing epoch seconds and milliseconds.
@@ -52,13 +54,14 @@ import java.util.Map;
  * The seconds formatter is provided by {@link #SECONDS_FORMATTER}.
  * The milliseconds formatter is provided by {@link #MILLIS_FORMATTER}.
  * <p>
- * Both formatters support fractional time, up to nanosecond precision. Values must be positive numbers.
+ * Both formatters support fractional time, up to nanosecond precision.
  */
 class EpochTime {
 
     private static final ValueRange LONG_POSITIVE_RANGE = ValueRange.of(0, Long.MAX_VALUE);
+    private static final ValueRange LONG_RANGE = ValueRange.of(Long.MIN_VALUE, Long.MAX_VALUE);
 
-    private static final EpochField SECONDS = new EpochField(ChronoUnit.SECONDS, ChronoUnit.FOREVER, LONG_POSITIVE_RANGE) {
+    private static final EpochField SECONDS = new EpochField(ChronoUnit.SECONDS, ChronoUnit.FOREVER, LONG_RANGE) {
         @Override
         public boolean isSupportedBy(TemporalAccessor temporal) {
             return temporal.isSupported(ChronoField.INSTANT_SECONDS);
@@ -97,15 +100,55 @@ class EpochTime {
         }
     };
 
-    private static final EpochField MILLIS = new EpochField(ChronoUnit.MILLIS, ChronoUnit.FOREVER, LONG_POSITIVE_RANGE) {
+    private static final long NEGATIVE = 0;
+    private static final long POSITIVE = 1;
+    private static final EpochField SIGN = new EpochField(ChronoUnit.FOREVER, ChronoUnit.FOREVER, ValueRange.of(NEGATIVE, POSITIVE)) {
         @Override
         public boolean isSupportedBy(TemporalAccessor temporal) {
-            return temporal.isSupported(ChronoField.INSTANT_SECONDS) && temporal.isSupported(ChronoField.MILLI_OF_SECOND);
+            return temporal.isSupported(ChronoField.INSTANT_SECONDS);
         }
 
         @Override
         public long getFrom(TemporalAccessor temporal) {
-            return temporal.getLong(ChronoField.INSTANT_SECONDS) * 1_000 + temporal.getLong(ChronoField.MILLI_OF_SECOND);
+            return temporal.getLong(ChronoField.INSTANT_SECONDS) < 0 ? NEGATIVE : POSITIVE;
+        }
+    };
+
+    // Millis as absolute values. Negative millis are encoded by having a NEGATIVE SIGN.
+    private static final EpochField MILLIS_ABS = new EpochField(ChronoUnit.MILLIS, ChronoUnit.FOREVER, LONG_POSITIVE_RANGE) {
+        @Override
+        public boolean isSupportedBy(TemporalAccessor temporal) {
+            return temporal.isSupported(ChronoField.INSTANT_SECONDS)
+                && (temporal.isSupported(ChronoField.NANO_OF_SECOND) || temporal.isSupported(ChronoField.MILLI_OF_SECOND));
+        }
+
+        @Override
+        public long getFrom(TemporalAccessor temporal) {
+            long instantSecondsInMillis = temporal.getLong(ChronoField.INSTANT_SECONDS) * 1_000;
+            if (instantSecondsInMillis >= 0) {
+                if (temporal.isSupported(ChronoField.NANO_OF_SECOND)) {
+                    return instantSecondsInMillis + (temporal.getLong(ChronoField.NANO_OF_SECOND) / 1_000_000);
+                } else {
+                    return instantSecondsInMillis + temporal.getLong(ChronoField.MILLI_OF_SECOND);
+                }
+            } else { // negative timestamp
+                if (temporal.isSupported(ChronoField.NANO_OF_SECOND)) {
+                    long millis = instantSecondsInMillis;
+                    long nanos = temporal.getLong(ChronoField.NANO_OF_SECOND);
+                    if (nanos % 1_000_000 != 0) {
+                        // Fractional negative timestamp.
+                        // Add 1 ms towards positive infinity because the fraction leads
+                        // the output's integral part to be an off-by-one when the
+                        // `(nanos / 1_000_000)` is added below.
+                        millis += 1;
+                    }
+                    millis += (nanos / 1_000_000);
+                    return -millis;
+                } else {
+                    long millisOfSecond = temporal.getLong(ChronoField.MILLI_OF_SECOND);
+                    return -(instantSecondsInMillis + millisOfSecond);
+                }
+            }
         }
 
         @Override
@@ -114,19 +157,47 @@ class EpochTime {
             TemporalAccessor partialTemporal,
             ResolverStyle resolverStyle
         ) {
-            long secondsAndMillis = fieldValues.remove(this);
+            Long sign = Optional.ofNullable(fieldValues.remove(SIGN)).orElse(POSITIVE);
-            long seconds = secondsAndMillis / 1_000;
+
-            long nanos = secondsAndMillis % 1000 * 1_000_000;
             Long nanosOfMilli = fieldValues.remove(NANOS_OF_MILLI);
+            long secondsAndMillis = fieldValues.remove(this);
+
+            long seconds;
+            long nanos;
+            if (sign == NEGATIVE) {
+                secondsAndMillis = -secondsAndMillis;
+                seconds = secondsAndMillis / 1_000;
+                nanos = secondsAndMillis % 1000 * 1_000_000;
+                // `secondsAndMillis < 0` implies negative timestamp; so `nanos < 0`
                 if (nanosOfMilli != null) {
+                    // aggregate fractional part of the input; subtract b/c `nanos < 0`
+                    nanos -= nanosOfMilli;
+                }
+                if (nanos != 0) {
+                    // nanos must be positive. B/c the timestamp is represented by the
+                    // (seconds, nanos) tuple, seconds moves 1s toward negative-infinity
+                    // and nanos moves 1s toward positive-infinity
+                    seconds -= 1;
+                    nanos = 1_000_000_000 + nanos;
+                }
+            } else {
+                seconds = secondsAndMillis / 1_000;
+                nanos = secondsAndMillis % 1000 * 1_000_000;
+
+                if (nanosOfMilli != null) {
+                    // aggregate fractional part of the input
                     nanos += nanosOfMilli;
                 }
+            }
             fieldValues.put(ChronoField.INSTANT_SECONDS, seconds);
             fieldValues.put(ChronoField.NANO_OF_SECOND, nanos);
             // if there is already a milli of second, we need to overwrite it
             if (fieldValues.containsKey(ChronoField.MILLI_OF_SECOND)) {
                 fieldValues.put(ChronoField.MILLI_OF_SECOND, nanos / 1_000_000);
             }
+            if (fieldValues.containsKey(ChronoField.MICRO_OF_SECOND)) {
+                fieldValues.put(ChronoField.MICRO_OF_SECOND, nanos / 1000);
+            }
             return null;
         }
     };
@@ -141,8 +212,12 @@ class EpochTime {
 
         @Override
         public long getFrom(TemporalAccessor temporal) {
+            if (temporal.getLong(ChronoField.INSTANT_SECONDS) < 0) {
+                return (1_000_000_000 - temporal.getLong(ChronoField.NANO_OF_SECOND)) % 1_000_000;
+            } else {
                 return temporal.getLong(ChronoField.NANO_OF_SECOND) % 1_000_000;
             }
+        }
     };
 
     // this supports seconds without any fraction
@@ -157,13 +232,22 @@ class EpochTime {
         .appendLiteral('.')
         .toFormatter(Locale.ROOT);
 
-    // this supports milliseconds without any fraction
+    private static final Map<Long, String> SIGN_FORMATTER_LOOKUP = new HashMap<Long, String>() {
-    private static final DateTimeFormatter MILLISECONDS_FORMATTER1 = new DateTimeFormatterBuilder().appendValue(
+        {
-        MILLIS,
+            put(POSITIVE, "");
-        1,
+            put(NEGATIVE, "-");
-        19,
+        }
-        SignStyle.NORMAL
+    };
-    ).optionalStart().appendFraction(NANOS_OF_MILLI, 0, 6, true).optionalEnd().toFormatter(Locale.ROOT);
+
+    // this supports milliseconds
+    private static final DateTimeFormatter MILLISECONDS_FORMATTER1 = new DateTimeFormatterBuilder().optionalStart()
+        .appendText(SIGN, SIGN_FORMATTER_LOOKUP) // field is only created in the presence of a '-' char.
+        .optionalEnd()
+        .appendValue(MILLIS_ABS, 1, 19, SignStyle.NOT_NEGATIVE)
+        .optionalStart()
+        .appendFraction(NANOS_OF_MILLI, 0, 6, true)
+        .optionalEnd()
+        .toFormatter(Locale.ROOT);
 
     // this supports milliseconds ending in dot
     private static final DateTimeFormatter MILLISECONDS_FORMATTER2 = new DateTimeFormatterBuilder().append(MILLISECONDS_FORMATTER1)

server/src/test/java/org/opensearch/common/time/DateFormattersTests.java

@@ -95,16 +95,127 @@ public class DateFormattersTests extends OpenSearchTestCase {
             Instant instant = Instant.from(formatter.parse("12345"));
             assertThat(instant.getEpochSecond(), is(12L));
             assertThat(instant.getNano(), is(345_000_000));
+            assertThat(formatter.format(instant), is("12345"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
         }
         {
             Instant instant = Instant.from(formatter.parse("0"));
             assertThat(instant.getEpochSecond(), is(0L));
             assertThat(instant.getNano(), is(0));
+            assertThat(formatter.format(instant), is("0"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-123000.123456"));
+            assertThat(instant.getEpochSecond(), is(-124L));
+            assertThat(instant.getNano(), is(999876544));
+            assertThat(formatter.format(instant), is("-123000.123456"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
         }
         {
             Instant instant = Instant.from(formatter.parse("123.123456"));
             assertThat(instant.getEpochSecond(), is(0L));
             assertThat(instant.getNano(), is(123123456));
+            assertThat(formatter.format(instant), is("123.123456"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-123.123456"));
+            assertThat(instant.getEpochSecond(), is(-1L));
+            assertThat(instant.getNano(), is(876876544));
+            assertThat(formatter.format(instant), is("-123.123456"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6789123.123456"));
+            assertThat(instant.getEpochSecond(), is(-6790L));
+            assertThat(instant.getNano(), is(876876544));
+            assertThat(formatter.format(instant), is("-6789123.123456"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("6789123.123456"));
+            assertThat(instant.getEpochSecond(), is(6789L));
+            assertThat(instant.getNano(), is(123123456));
+            assertThat(formatter.format(instant), is("6789123.123456"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000430768.25"));
+            assertThat(instant.getEpochSecond(), is(-6250000431L));
+            assertThat(instant.getNano(), is(231750000));
+            assertThat(formatter.format(instant), is("-6250000430768.25"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000430768.75"));
+            assertThat(instant.getEpochSecond(), is(-6250000431L));
+            assertThat(instant.getNano(), is(231250000));
+            assertThat(formatter.format(instant), is("-6250000430768.75"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000430768.00"));
+            assertThat(instant.getEpochSecond(), is(-6250000431L));
+            assertThat(instant.getNano(), is(232000000));
+            assertThat(formatter.format(instant), is("-6250000430768")); // remove .00 precision
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000431000.250000"));
+            assertThat(instant.getEpochSecond(), is(-6250000432L));
+            assertThat(instant.getNano(), is(999750000));
+            assertThat(formatter.format(instant), is("-6250000431000.25"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000431000.000001"));
+            assertThat(instant.getEpochSecond(), is(-6250000432L));
+            assertThat(instant.getNano(), is(999999999));
+            assertThat(formatter.format(instant), is("-6250000431000.000001"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000431000.75"));
+            assertThat(instant.getEpochSecond(), is(-6250000432L));
+            assertThat(instant.getNano(), is(999250000));
+            assertThat(formatter.format(instant), is("-6250000431000.75"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000431000.00"));
+            assertThat(instant.getEpochSecond(), is(-6250000431L));
+            assertThat(instant.getNano(), is(0));
+            assertThat(formatter.format(instant), is("-6250000431000"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000431000"));
+            assertThat(instant.getEpochSecond(), is(-6250000431L));
+            assertThat(instant.getNano(), is(0));
+            assertThat(formatter.format(instant), is("-6250000431000"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-6250000430768"));
+            assertThat(instant.getEpochSecond(), is(-6250000431L));
+            assertThat(instant.getNano(), is(232000000));
+            assertThat(formatter.format(instant), is("-6250000430768"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("1680000430768"));
+            assertThat(instant.getEpochSecond(), is(1680000430L));
+            assertThat(instant.getNano(), is(768000000));
+            assertThat(formatter.format(instant), is("1680000430768"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
+        }
+        {
+            Instant instant = Instant.from(formatter.parse("-0.12345"));
+            assertThat(instant.getEpochSecond(), is(-1L));
+            assertThat(instant.getNano(), is(999876550));
+            assertThat(formatter.format(instant), is("-0.12345"));
+            assertThat(Instant.from(formatter.parse(formatter.format(instant))), is(instant));
         }
     }
 
@@ -227,7 +338,7 @@ public class DateFormattersTests extends OpenSearchTestCase {
         long seconds = randomLongBetween(0, 130L * 365 * 86400); // from 1970 epoch till around 2100
         long nanos = randomLongBetween(0, 999_999_999L);
         Instant instant = Instant.ofEpochSecond(seconds, nanos);
-
+        {
             DateFormatter millisFormatter = DateFormatter.forPattern("epoch_millis");
             String millis = millisFormatter.format(instant);
             Instant millisInstant = Instant.from(millisFormatter.parse(millis));
@@ -242,6 +353,55 @@ public class DateFormattersTests extends OpenSearchTestCase {
 
             assertThat(secondsFormatter.format(Instant.ofEpochSecond(42, 0)), is("42"));
         }
+        {
+            DateFormatter isoFormatter = DateFormatters.forPattern("strict_date_optional_time_nanos");
+            DateFormatter millisFormatter = DateFormatter.forPattern("epoch_millis");
+            String millis = millisFormatter.format(instant);
+            String iso8601 = isoFormatter.format(instant);
+
+            Instant millisInstant = Instant.from(millisFormatter.parse(millis));
+            Instant isoInstant = Instant.from(isoFormatter.parse(iso8601));
+
+            assertThat(millisInstant.toEpochMilli(), is(isoInstant.toEpochMilli()));
+            assertThat(millisInstant.getEpochSecond(), is(isoInstant.getEpochSecond()));
+            assertThat(millisInstant.getNano(), is(isoInstant.getNano()));
+        }
+    }
+
+    public void testEpochFormattingNegativeEpoch() {
+        long seconds = randomLongBetween(-130L * 365 * 86400, 0); // around 1840 till 1970 epoch
+        long nanos = randomLongBetween(0, 999_999_999L);
+        Instant instant = Instant.ofEpochSecond(seconds, nanos);
+
+        {
+            DateFormatter millisFormatter = DateFormatter.forPattern("epoch_millis");
+            String millis = millisFormatter.format(instant);
+            Instant millisInstant = Instant.from(millisFormatter.parse(millis));
+            assertThat(millisInstant.toEpochMilli(), is(instant.toEpochMilli()));
+            assertThat(millisFormatter.format(Instant.ofEpochSecond(-42, 0)), is("-42000"));
+            assertThat(millisFormatter.format(Instant.ofEpochSecond(-42, 123456789L)), is("-41876.543211"));
+
+            DateFormatter secondsFormatter = DateFormatter.forPattern("epoch_second");
+            String formattedSeconds = secondsFormatter.format(instant);
+            Instant secondsInstant = Instant.from(secondsFormatter.parse(formattedSeconds));
+            assertThat(secondsInstant.getEpochSecond(), is(instant.getEpochSecond()));
+
+            assertThat(secondsFormatter.format(Instant.ofEpochSecond(42, 0)), is("42"));
+        }
+        {
+            DateFormatter isoFormatter = DateFormatters.forPattern("strict_date_optional_time_nanos");
+            DateFormatter millisFormatter = DateFormatter.forPattern("epoch_millis");
+            String millis = millisFormatter.format(instant);
+            String iso8601 = isoFormatter.format(instant);
+
+            Instant millisInstant = Instant.from(millisFormatter.parse(millis));
+            Instant isoInstant = Instant.from(isoFormatter.parse(iso8601));
+
+            assertThat(millisInstant.toEpochMilli(), is(isoInstant.toEpochMilli()));
+            assertThat(millisInstant.getEpochSecond(), is(isoInstant.getEpochSecond()));
+            assertThat(millisInstant.getNano(), is(isoInstant.getNano()));
+        }
+    }
 
     public void testParsingStrictNanoDates() {
         DateFormatter formatter = DateFormatters.forPattern("strict_date_optional_time_nanos");