From 4b89410055c4788f32058e74f7f3161ef783f44e Mon Sep 17 00:00:00 2001 From: Breno Faria <30877433+br3no@users.noreply.github.com> Date: Wed, 2 Mar 2022 19:52:38 +0100 Subject: [PATCH] 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 * applying spotless fix Signed-off-by: Breno Faria * more conservative implementation of isSupportedBy Signed-off-by: Breno Faria * adding braces to control flow statement Signed-off-by: Breno Faria * spotless fix... Signed-off-by: Breno Faria Co-authored-by: Breno Faria --- .../org/opensearch/common/time/EpochTime.java | 120 ++++++++++-- .../common/time/DateFormattersTests.java | 182 ++++++++++++++++-- 2 files changed, 273 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/org/opensearch/common/time/EpochTime.java b/server/src/main/java/org/opensearch/common/time/EpochTime.java index 5c6e024c747..7894a653492 100644 --- a/server/src/main/java/org/opensearch/common/time/EpochTime.java +++ b/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}. *

- * 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,12 +157,37 @@ class EpochTime { TemporalAccessor partialTemporal, ResolverStyle resolverStyle ) { - long secondsAndMillis = fieldValues.remove(this); - long seconds = secondsAndMillis / 1_000; - long nanos = secondsAndMillis % 1000 * 1_000_000; + Long sign = Optional.ofNullable(fieldValues.remove(SIGN)).orElse(POSITIVE); + Long nanosOfMilli = fieldValues.remove(NANOS_OF_MILLI); - if (nanosOfMilli != null) { - nanos += nanosOfMilli; + 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); @@ -127,6 +195,9 @@ class EpochTime { 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,7 +212,11 @@ class EpochTime { @Override public long getFrom(TemporalAccessor temporal) { - return temporal.getLong(ChronoField.NANO_OF_SECOND) % 1_000_000; + 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; + } } }; @@ -157,13 +232,22 @@ class EpochTime { .appendLiteral('.') .toFormatter(Locale.ROOT); - // this supports milliseconds without any fraction - private static final DateTimeFormatter MILLISECONDS_FORMATTER1 = new DateTimeFormatterBuilder().appendValue( - MILLIS, - 1, - 19, - SignStyle.NORMAL - ).optionalStart().appendFraction(NANOS_OF_MILLI, 0, 6, true).optionalEnd().toFormatter(Locale.ROOT); + private static final Map SIGN_FORMATTER_LOOKUP = new HashMap() { + { + put(POSITIVE, ""); + put(NEGATIVE, "-"); + } + }; + + // 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) diff --git a/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java b/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java index b67372ea9e8..1e57f9fe88d 100644 --- a/server/src/test/java/org/opensearch/common/time/DateFormattersTests.java +++ b/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,20 +338,69 @@ 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)); + assertThat(millisInstant.toEpochMilli(), is(instant.toEpochMilli())); + assertThat(millisFormatter.format(Instant.ofEpochSecond(42, 0)), is("42000")); + assertThat(millisFormatter.format(Instant.ofEpochSecond(42, 123456789L)), is("42123.456789")); - 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("42123.456789")); + DateFormatter secondsFormatter = DateFormatter.forPattern("epoch_second"); + String formattedSeconds = secondsFormatter.format(instant); + Instant secondsInstant = Instant.from(secondsFormatter.parse(formattedSeconds)); + assertThat(secondsInstant.getEpochSecond(), is(instant.getEpochSecond())); - 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); - assertThat(secondsFormatter.format(Instant.ofEpochSecond(42, 0)), is("42")); + 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() {