Skip to content

Commit

Permalink
Core: Deprecate negative epoch timestamps (#36793)
Browse files Browse the repository at this point in the history
Negative timestamps are currently supported in joda time. These are
dates before epoch. However, it doesn't really make sense to have a
negative timestamp, since this is a modern format. Any dates before
epoch can be represented with normal date formats, like ISO8601.
Additionally, implementing negative epoch timestamp parsing in java time
has an edge case which would more than double the code required. This
commit deprecates use of negative epoch timestamps.
  • Loading branch information
rjernst committed Dec 20, 2018
1 parent 29fa1a9 commit 0d2528c
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ setup:
index: test
type: test
id: 4
body: { "date" : "-2524492800000" }
body: { "date" : "10000" }

- do:
index:
Expand All @@ -322,16 +322,16 @@ setup:
age_groups:
date_range:
field: date
missing: "-2240496000000"
missing: "0"
ranges:
- key: Generation Y
from: '315561600000'
to: '946713600000'
- key: Generation X
from: "-157737600000"
from: "200000"
to: '315561600000'
- key: Other
to: "-2208960000000"
to: "200000"

- match: { hits.total: 5 }

Expand Down
8 changes: 6 additions & 2 deletions server/src/main/java/org/elasticsearch/common/joda/Joda.java
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,14 @@ public int parseInto(DateTimeParserBucket bucket, String text, int position) {
int factor = hasMilliSecondPrecision ? 1 : 1000;
try {
long millis = new BigDecimal(text).longValue() * factor;
// check for deprecation, but after it has parsed correctly so the "e" isn't from something else
// check for deprecations, but after it has parsed correctly so invalid values aren't counted as deprecated
if (millis < 0) {
deprecationLogger.deprecatedAndMaybeLog("epoch-negative", "Use of negative values" +
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
}
if (scientificNotation.matcher(text).find()) {
deprecationLogger.deprecatedAndMaybeLog("epoch-scientific-notation", "Use of scientific notation" +
"in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
}
DateTime dt = new DateTime(millis, DateTimeZone.UTC);
bucket.saveField(DateTimeFieldType.year(), dt.getYear());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,9 @@ public void testDuellingFormatsValidParsing() {
assertSameDate("1522332219.0", "epoch_second");
assertSameDate("0", "epoch_second");
assertSameDate("1", "epoch_second");
assertSameDate("-1", "epoch_second");
assertSameDate("-1522332219", "epoch_second");
assertSameDate("1522332219321", "epoch_millis");
assertSameDate("0", "epoch_millis");
assertSameDate("1", "epoch_millis");
assertSameDate("-1", "epoch_millis");
assertSameDate("-1522332219321", "epoch_millis");

assertSameDate("20181126", "basic_date");
assertSameDate("20181126T121212.123Z", "basic_date_time");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ public void testThatNegativeEpochsCanBeParsed() {
formatter.parseJoda("-1234567890.9999");
formatter.parseJoda("-1234567890123456.9999");
}

assertWarnings("Use of negative values" +
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
}

public void testForInvalidDatesInEpochSecond() {
Expand Down Expand Up @@ -753,13 +756,22 @@ public void testDeprecatedFormatSpecifiers() {
" next major version of Elasticsearch. Prefix your date format with '8' to use the new specifier.");
}

public void testDeprecatedScientificNotation() {
public void testDeprecatedEpochScientificNotation() {
assertValidDateFormatParsing("epoch_second", "1.234e5", "123400");
assertWarnings("Use of scientific notation" +
"in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
assertValidDateFormatParsing("epoch_millis", "1.234e5", "123400");
assertWarnings("Use of scientific notation" +
"in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
}

public void testDeprecatedEpochNegative() {
assertValidDateFormatParsing("epoch_second", "-12345", "-12345");
assertWarnings("Use of negative values" +
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
assertValidDateFormatParsing("epoch_millis", "-12345", "-12345");
assertWarnings("Use of negative values" +
" in epoch time formats is deprecated and will not be supported in the next major version of Elasticsearch.");
}

private void assertValidDateFormatParsing(String pattern, String dateToParse) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@

import java.io.IOException;
import java.util.Collection;
import java.util.Locale;

import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.notNullValue;
Expand Down Expand Up @@ -228,8 +227,8 @@ public void testFloatEpochFormat() throws IOException {

assertEquals(mapping, mapper.mappingSource().toString());

double epochFloatMillisFromEpoch = (randomDouble() * 2 - 1) * 1000000;
String epochFloatValue = String.format(Locale.US, "%f", epochFloatMillisFromEpoch);
long epochMillis = randomNonNegativeLong();
String epochFloatValue = epochMillis + "." + randomIntBetween(0, 999);

ParsedDocument doc = mapper.parse(SourceToParse.source("test", "type", "1", BytesReference
.bytes(XContentFactory.jsonBuilder()
Expand All @@ -241,7 +240,7 @@ public void testFloatEpochFormat() throws IOException {
IndexableField[] fields = doc.rootDoc().getFields("field");
assertEquals(2, fields.length);
IndexableField pointField = fields[0];
assertEquals((long)epochFloatMillisFromEpoch, pointField.numericValue().longValue());
assertEquals(epochMillis, pointField.numericValue().longValue());
}

public void testChangeLocale() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void testGettingInvalidByte() throws Exception {
double doubleNotByte = randomDoubleBetween(Byte.MAX_VALUE + 1, Double.MAX_VALUE, true);
float floatNotByte = randomFloatBetween(Byte.MAX_VALUE + 1, Float.MAX_VALUE);
String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);
long randomDate = randomLong();
long randomDate = randomNonNegativeLong();

String doubleErrorMessage = (doubleNotByte > Long.MAX_VALUE || doubleNotByte < Long.MIN_VALUE) ?
Double.toString(doubleNotByte) : Long.toString(Math.round(doubleNotByte));
Expand Down Expand Up @@ -279,7 +279,7 @@ public void testGettingInvalidShort() throws Exception {
double doubleNotShort = randomDoubleBetween(Short.MAX_VALUE + 1, Double.MAX_VALUE, true);
float floatNotShort = randomFloatBetween(Short.MAX_VALUE + 1, Float.MAX_VALUE);
String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);
long randomDate = randomLong();
long randomDate = randomNonNegativeLong();

String doubleErrorMessage = (doubleNotShort > Long.MAX_VALUE || doubleNotShort < Long.MIN_VALUE) ?
Double.toString(doubleNotShort) : Long.toString(Math.round(doubleNotShort));
Expand Down Expand Up @@ -400,7 +400,7 @@ public void testGettingInvalidInteger() throws Exception {
double doubleNotInt = randomDoubleBetween(getMaxIntPlusOne().doubleValue(), Double.MAX_VALUE, true);
float floatNotInt = randomFloatBetween(getMaxIntPlusOne().floatValue(), Float.MAX_VALUE);
String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);
long randomDate = randomLong();
long randomDate = randomNonNegativeLong();

String doubleErrorMessage = (doubleNotInt > Long.MAX_VALUE || doubleNotInt < Long.MIN_VALUE) ?
Double.toString(doubleNotInt) : Long.toString(Math.round(doubleNotInt));
Expand Down Expand Up @@ -511,7 +511,7 @@ public void testGettingInvalidLong() throws Exception {
double doubleNotLong = randomDoubleBetween(getMaxLongPlusOne().doubleValue(), Double.MAX_VALUE, true);
float floatNotLong = randomFloatBetween(getMaxLongPlusOne().floatValue(), Float.MAX_VALUE);
String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);
long randomDate = randomLong();
long randomDate = randomNonNegativeLong();

index("test", "1", builder -> {
builder.field("test_double", doubleNotLong);
Expand Down Expand Up @@ -606,7 +606,7 @@ public void testGettingInvalidDouble() throws Exception {
});

String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);
long randomDate = randomLong();
long randomDate = randomNonNegativeLong();

index("test", "1", builder -> {
builder.field("test_keyword", randomString);
Expand Down Expand Up @@ -689,7 +689,7 @@ public void testGettingInvalidFloat() throws Exception {
});

String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);
long randomDate = randomLong();
long randomDate = randomNonNegativeLong();

index("test", "1", builder -> {
builder.field("test_keyword", randomString);
Expand Down Expand Up @@ -722,8 +722,8 @@ public void testGettingBooleanValues() throws Exception {
builder.startObject("test_boolean").field("type", "boolean").endObject();
builder.startObject("test_date").field("type", "date").endObject();
});
long randomDate1 = randomLong();
long randomDate2 = randomLong();
long randomDate1 = randomNonNegativeLong();
long randomDate2 = randomNonNegativeLong();

// true values
indexSimpleDocumentWithTrueValues(randomDate1);
Expand Down Expand Up @@ -805,7 +805,7 @@ public void testGettingDateWithoutCalendar() throws Exception {
builder.startObject("test_boolean").field("type", "boolean").endObject();
builder.startObject("test_date").field("type", "date").endObject();
});
Long randomLongDate = randomLong();
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);

String timeZoneId = randomKnownTimeZone();
Expand Down Expand Up @@ -838,7 +838,7 @@ public void testGettingDateWithCalendar() throws Exception {
builder.startObject("test_boolean").field("type", "boolean").endObject();
builder.startObject("test_date").field("type", "date").endObject();
});
Long randomLongDate = randomLong();
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);
index("test", "2", builder -> {
builder.timeField("test_date", null);
Expand Down Expand Up @@ -874,7 +874,7 @@ public void testGettingTimeWithoutCalendar() throws Exception {
builder.startObject("test_boolean").field("type", "boolean").endObject();
builder.startObject("test_date").field("type", "date").endObject();
});
Long randomLongDate = randomLong();
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);

String timeZoneId = randomKnownTimeZone();
Expand Down Expand Up @@ -906,7 +906,7 @@ public void testGettingTimeWithCalendar() throws Exception {
builder.startObject("test_boolean").field("type", "boolean").endObject();
builder.startObject("test_date").field("type", "date").endObject();
});
Long randomLongDate = randomLong();
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);
index("test", "2", builder -> {
builder.timeField("test_date", null);
Expand Down Expand Up @@ -940,7 +940,7 @@ public void testGettingTimestampWithoutCalendar() throws Exception {
builder.startObject("release_date").field("type", "date").endObject();
builder.startObject("republish_date").field("type", "date").endObject();
});
long randomMillis = randomLong();
long randomMillis = randomNonNegativeLong();

index("library", "1", builder -> {
builder.field("name", "Don Quixote");
Expand All @@ -951,7 +951,7 @@ public void testGettingTimestampWithoutCalendar() throws Exception {
index("library", "2", builder -> {
builder.field("name", "1984");
builder.field("page_count", 328);
builder.field("release_date", -649036800000L);
builder.field("release_date", 649036800000L);
builder.field("republish_date", 599616000000L);
});

Expand All @@ -970,7 +970,7 @@ public void testGettingTimestampWithoutCalendar() throws Exception {

assertTrue(results.next());
assertEquals(599616000000L, results.getTimestamp("republish_date").getTime());
assertEquals(-649036800000L, ((Timestamp) results.getObject(2)).getTime());
assertEquals(649036800000L, ((Timestamp) results.getObject(2)).getTime());

assertFalse(results.next());
});
Expand All @@ -983,7 +983,7 @@ public void testGettingTimestampWithCalendar() throws Exception {
builder.startObject("test_boolean").field("type", "boolean").endObject();
builder.startObject("test_date").field("type", "date").endObject();
});
Long randomLongDate = randomLong();
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);
index("test", "2", builder -> {
builder.timeField("test_date", null);
Expand Down Expand Up @@ -1022,7 +1022,7 @@ public void testValidGetObjectCalls() throws Exception {
double d = randomDouble();
float f = randomFloat();
boolean randomBool = randomBoolean();
Long randomLongDate = randomLong();
Long randomLongDate = randomNonNegativeLong();
String randomString = randomUnicodeOfCodepointLengthBetween(128, 256);

index("test", "1", builder -> {
Expand Down

0 comments on commit 0d2528c

Please sign in to comment.