From c1e5421b7718a918885e7e5f22ab4e0b7bbbdfc6 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 7 Oct 2016 14:22:15 +0200 Subject: [PATCH] Make range queries round up upper bounds again. (#20582) Elasticsearch 1.x used to implicitly round up upper bounds of queries when they were inclusive so that eg. `[2016-09-18 TO 2016-09-20]` would actually run `[2016-09-18T00:00:00.000Z TO 2016-09-20T23:59:59.999Z]` and include dates like `2016-09-20T15:32:44`. This behaviour was lost in the cleanups of #8889. Closes #20579 --- .../common/joda/DateMathParser.java | 27 +++++++---- .../common/joda/DateMathParserTests.java | 48 +++++++++++++++++-- .../index/mapper/DateFieldTypeTests.java | 24 +++++----- .../mapper/LegacyDateFieldMapperTests.java | 4 +- .../mapper/LegacyDateFieldTypeTests.java | 2 +- 5 files changed, 80 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/joda/DateMathParser.java b/core/src/main/java/org/elasticsearch/common/joda/DateMathParser.java index 94ac18fbe95..ba5531c813c 100644 --- a/core/src/main/java/org/elasticsearch/common/joda/DateMathParser.java +++ b/core/src/main/java/org/elasticsearch/common/joda/DateMathParser.java @@ -63,13 +63,10 @@ public class DateMathParser { } else { int index = text.indexOf("||"); if (index == -1) { - return parseDateTime(text, timeZone); + return parseDateTime(text, timeZone, roundUp); } - time = parseDateTime(text.substring(0, index), timeZone); + time = parseDateTime(text.substring(0, index), timeZone, false); mathString = text.substring(index + 2); - if (mathString.isEmpty()) { - return time; - } } return parseMath(mathString, time, roundUp, timeZone); @@ -190,15 +187,29 @@ public class DateMathParser { return dateTime.getMillis(); } - private long parseDateTime(String value, DateTimeZone timeZone) { + private long parseDateTime(String value, DateTimeZone timeZone, boolean roundUpIfNoTime) { DateTimeFormatter parser = dateTimeFormatter.parser(); if (timeZone != null) { parser = parser.withZone(timeZone); } try { - return parser.parseMillis(value); + MutableDateTime date; + // We use 01/01/1970 as a base date so that things keep working with date + // fields that are filled with times without dates + if (roundUpIfNoTime) { + date = new MutableDateTime(1970, 1, 1, 23, 59, 59, 999, DateTimeZone.UTC); + } else { + date = new MutableDateTime(1970, 1, 1, 0, 0, 0, 0, DateTimeZone.UTC); + } + final int end = parser.parseInto(date, value, 0); + if (end < 0) { + int position = ~end; + throw new IllegalArgumentException("Parse failure at index [" + position + "] of [" + value + "]"); + } else if (end != value.length()) { + throw new IllegalArgumentException("Unrecognized chars at the end of [" + value + "]: [" + value.substring(end) + "]"); + } + return date.getMillis(); } catch (IllegalArgumentException e) { - throw new ElasticsearchParseException("failed to parse date field [{}] with format [{}]", e, value, dateTimeFormatter.format()); } } diff --git a/core/src/test/java/org/elasticsearch/common/joda/DateMathParserTests.java b/core/src/test/java/org/elasticsearch/common/joda/DateMathParserTests.java index cac1335dbdc..505196a97f6 100644 --- a/core/src/test/java/org/elasticsearch/common/joda/DateMathParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/joda/DateMathParserTests.java @@ -68,8 +68,14 @@ public class DateMathParserTests extends ESTestCase { } public void testRoundingDoesNotAffectExactDate() { - assertDateMathEquals("2014-11-12T22:55:00Z", "2014-11-12T22:55:00Z", 0, true, null); - assertDateMathEquals("2014-11-12T22:55:00Z", "2014-11-12T22:55:00Z", 0, false, null); + assertDateMathEquals("2014-11-12T22:55:00.000Z", "2014-11-12T22:55:00.000Z", 0, true, null); + assertDateMathEquals("2014-11-12T22:55:00.000Z", "2014-11-12T22:55:00.000Z", 0, false, null); + + assertDateMathEquals("2014-11-12T22:55:00.000", "2014-11-12T21:55:00.000Z", 0, true, DateTimeZone.forID("+01:00")); + assertDateMathEquals("2014-11-12T22:55:00.000", "2014-11-12T21:55:00.000Z", 0, false, DateTimeZone.forID("+01:00")); + + assertDateMathEquals("2014-11-12T22:55:00.000+01:00", "2014-11-12T21:55:00.000Z", 0, true, null); + assertDateMathEquals("2014-11-12T22:55:00.000+01:00", "2014-11-12T21:55:00.000Z", 0, false, null); } public void testTimezone() { @@ -134,7 +140,43 @@ public class DateMathParserTests extends ESTestCase { assertDateMathEquals("now/m", "2014-11-18T14:27", now, false, DateTimeZone.forID("+02:00")); } - public void testRounding() { + public void testRoundingPreservesEpochAsBaseDate() { + // If a user only specifies times, then the date needs to always be 1970-01-01 regardless of rounding + FormatDateTimeFormatter formatter = Joda.forPattern("HH:mm:ss"); + DateMathParser parser = new DateMathParser(formatter); + assertEquals( + this.formatter.parser().parseMillis("1970-01-01T04:52:20.000Z"), + parser.parse("04:52:20", () -> 0, false, null)); + assertEquals( + this.formatter.parser().parseMillis("1970-01-01T04:52:20.999Z"), + parser.parse("04:52:20", () -> 0, true, null)); + } + + // Implicit rounding happening when parts of the date are not specified + public void testImplicitRounding() { + assertDateMathEquals("2014-11-18", "2014-11-18", 0, false, null); + assertDateMathEquals("2014-11-18", "2014-11-18T23:59:59.999Z", 0, true, null); + + assertDateMathEquals("2014-11-18T09:20", "2014-11-18T09:20", 0, false, null); + assertDateMathEquals("2014-11-18T09:20", "2014-11-18T09:20:59.999Z", 0, true, null); + + assertDateMathEquals("2014-11-18", "2014-11-17T23:00:00.000Z", 0, false, DateTimeZone.forID("CET")); + assertDateMathEquals("2014-11-18", "2014-11-18T22:59:59.999Z", 0, true, DateTimeZone.forID("CET")); + + assertDateMathEquals("2014-11-18T09:20", "2014-11-18T08:20:00.000Z", 0, false, DateTimeZone.forID("CET")); + assertDateMathEquals("2014-11-18T09:20", "2014-11-18T08:20:59.999Z", 0, true, DateTimeZone.forID("CET")); + + // implicit rounding with explicit timezone in the date format + FormatDateTimeFormatter formatter = Joda.forPattern("YYYY-MM-ddZ"); + DateMathParser parser = new DateMathParser(formatter); + long time = parser.parse("2011-10-09+01:00", () -> 0, false, null); + assertEquals(this.parser.parse("2011-10-09T00:00:00.000+01:00", () -> 0), time); + time = parser.parse("2011-10-09+01:00", () -> 0, true, null); + assertEquals(this.parser.parse("2011-10-09T23:59:59.999+01:00", () -> 0), time); + } + + // Explicit rounding using the || separator + public void testExplicitRounding() { assertDateMathEquals("2014-11-18||/y", "2014-01-01", 0, false, null); assertDateMathEquals("2014-11-18||/y", "2014-12-31T23:59:59.999", 0, true, null); assertDateMathEquals("2014||/y", "2014-01-01", 0, false, null); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java index 377b8d2da77..12fd641724e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DateFieldTypeTests.java @@ -36,6 +36,8 @@ import org.elasticsearch.common.joda.DateMathParser; import org.elasticsearch.common.joda.Joda; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexSettings; +import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.DateFieldMapper.DateFieldType; import org.elasticsearch.index.mapper.MappedFieldType.Relation; import org.elasticsearch.index.mapper.ParseContext.Document; @@ -106,8 +108,8 @@ public class DateFieldTypeTests extends FieldTypeTestCase { public void testIsFieldWithinQuery() throws IOException { Directory dir = newDirectory(); IndexWriter w = new IndexWriter(dir, new IndexWriterConfig(null)); - long instant1 = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-12").getMillis(); - long instant2 = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime("2016-04-03").getMillis(); + long instant1 = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-12").getMillis(); + long instant2 = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime("2016-04-03").getMillis(); Document doc = new Document(); LongPoint field = new LongPoint("my_date", instant1); doc.add(field); @@ -117,7 +119,7 @@ public class DateFieldTypeTests extends FieldTypeTestCase { DirectoryReader reader = DirectoryReader.open(w); DateFieldType ft = new DateFieldType(); ft.setName("my_date"); - DateMathParser alternateFormat = new DateMathParser(LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER); + DateMathParser alternateFormat = new DateMathParser(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER); doTestIsFieldWithinQuery(ft, reader, null, null); doTestIsFieldWithinQuery(ft, reader, null, alternateFormat); doTestIsFieldWithinQuery(ft, reader, DateTimeZone.UTC, null); @@ -132,7 +134,7 @@ public class DateFieldTypeTests extends FieldTypeTestCase { public void testValueFormat() { MappedFieldType ft = createDefaultFieldType(); - long instant = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-12T14:10:55").getMillis(); + long instant = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-12T14:10:55").getMillis(); assertEquals("2015-10-12T14:10:55.000Z", ft.docValueFormat(null, DateTimeZone.UTC).format(instant)); assertEquals("2015-10-12T15:10:55.000+01:00", @@ -141,16 +143,16 @@ public class DateFieldTypeTests extends FieldTypeTestCase { createDefaultFieldType().docValueFormat("YYYY", DateTimeZone.UTC).format(instant)); assertEquals(instant, ft.docValueFormat(null, DateTimeZone.UTC).parseLong("2015-10-12T14:10:55", false, null)); - assertEquals(instant, + assertEquals(instant + 999, ft.docValueFormat(null, DateTimeZone.UTC).parseLong("2015-10-12T14:10:55", true, null)); - assertEquals(LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-13").getMillis() - 1, + assertEquals(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-13").getMillis() - 1, ft.docValueFormat(null, DateTimeZone.UTC).parseLong("2015-10-12||/d", true, null)); } public void testValueForSearch() { MappedFieldType ft = createDefaultFieldType(); String date = "2015-10-12T12:09:55.000Z"; - long instant = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime(date).getMillis(); + long instant = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime(date).getMillis(); assertEquals(date, ft.valueForDisplay(instant)); } @@ -164,9 +166,9 @@ public class DateFieldTypeTests extends FieldTypeTestCase { MappedFieldType ft = createDefaultFieldType(); ft.setName("field"); String date = "2015-10-12T14:10:55"; - long instant = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime(date).getMillis(); + long instant = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime(date).getMillis(); ft.setIndexOptions(IndexOptions.DOCS); - assertEquals(LongPoint.newExactQuery("field", instant), ft.termQuery(date, context)); + assertEquals(LongPoint.newRangeQuery("field", instant, instant + 999), ft.termQuery(date, context)); ft.setIndexOptions(IndexOptions.NONE); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, @@ -184,8 +186,8 @@ public class DateFieldTypeTests extends FieldTypeTestCase { ft.setName("field"); String date1 = "2015-10-12T14:10:55"; String date2 = "2016-04-28T11:33:52"; - long instant1 = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime(date1).getMillis(); - long instant2 = LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime(date2).getMillis(); + long instant1 = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime(date1).getMillis(); + long instant2 = DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER.parser().parseDateTime(date2).getMillis() + 999; ft.setIndexOptions(IndexOptions.DOCS); assertEquals(LongPoint.newRangeQuery("field", instant1, instant2), ft.rangeQuery(date1, date2, true, true, context).rewrite(new MultiReader())); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldMapperTests.java b/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldMapperTests.java index cea33ab51ec..3d644a01af1 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldMapperTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldMapperTests.java @@ -257,7 +257,7 @@ public class LegacyDateFieldMapperTests extends ESSingleNodeTestCase { LegacyNumericRangeQuery rangeQuery = (LegacyNumericRangeQuery) defaultMapper.mappers().smartNameFieldMapper("date_field").fieldType() .rangeQuery("10:00:00", "11:00:00", true, true, context).rewrite(null); - assertThat(rangeQuery.getMax(), equalTo(new DateTime(TimeValue.timeValueHours(11).millis(), DateTimeZone.UTC).getMillis())); + assertThat(rangeQuery.getMax(), equalTo(new DateTime(TimeValue.timeValueHours(11).millis(), DateTimeZone.UTC).getMillis() + 999)); assertThat(rangeQuery.getMin(), equalTo(new DateTime(TimeValue.timeValueHours(10).millis(), DateTimeZone.UTC).getMillis())); } @@ -284,7 +284,7 @@ public class LegacyDateFieldMapperTests extends ESSingleNodeTestCase { LegacyNumericRangeQuery rangeQuery = (LegacyNumericRangeQuery) defaultMapper.mappers().smartNameFieldMapper("date_field").fieldType() .rangeQuery("Jan 02 10:00:00", "Jan 02 11:00:00", true, true, context).rewrite(null); - assertThat(rangeQuery.getMax(), equalTo(new DateTime(TimeValue.timeValueHours(35).millis(), DateTimeZone.UTC).getMillis())); + assertThat(rangeQuery.getMax(), equalTo(new DateTime(TimeValue.timeValueHours(35).millis() + 999, DateTimeZone.UTC).getMillis())); assertThat(rangeQuery.getMin(), equalTo(new DateTime(TimeValue.timeValueHours(34).millis(), DateTimeZone.UTC).getMillis())); } diff --git a/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldTypeTests.java b/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldTypeTests.java index 243df8c7553..10a2a331a79 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldTypeTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/LegacyDateFieldTypeTests.java @@ -138,7 +138,7 @@ public class LegacyDateFieldTypeTests extends FieldTypeTestCase { createDefaultFieldType().docValueFormat("YYYY", DateTimeZone.UTC).format(instant)); assertEquals(instant, ft.docValueFormat(null, DateTimeZone.UTC).parseLong("2015-10-12T14:10:55", false, null)); - assertEquals(instant, + assertEquals(instant + 999, ft.docValueFormat(null, DateTimeZone.UTC).parseLong("2015-10-12T14:10:55", true, null)); assertEquals(LegacyDateFieldMapper.Defaults.DATE_TIME_FORMATTER.parser().parseDateTime("2015-10-13").getMillis() - 1, ft.docValueFormat(null, DateTimeZone.UTC).parseLong("2015-10-12||/d", true, null));