DateMath: Fix semantics of rounding with inclusive/exclusive ranges.

Date math rounding currently works by rounding the date up or down based
on the scope of the rounding.  For example, if you have the date
`2009-12-24||/d` it will round down to the inclusive lower end
`2009-12-24T00:00:00.000` and round up to the non-inclusive date
`2009-12-25T00:00:00.000`.

The range endpoint semantics work as follows:
* `gt` - round D down, and use > that value
* `gte` - round D down, and use >= that value
* `lt` - round D down, and use <
* `lte` - round D up, and use <=

There are 2 problems with these semantics:
* `lte` ends up including the upper value, which should be non-inclusive
* `gt` only excludes the beginning of the date, not the entire rounding scope

This change makes the range endpoint semantics symmetrical.  First, it
changes the parser to round up and down using the first (same as before)
and last (1 ms less than before) values of the rounding scope.  This
makes both rounded endpoints inclusive. The range endpoint semantics
are then as follows:
* `gt` - round D up, and use > that value
* `gte` - round D down, and use >= that value
* `lt` - round D down, and use < that value
* `lte` - round D up, and use <= that value

closes #8424
closes #8556
This commit is contained in:
Ryan Ernst 2014-11-19 09:10:06 -08:00
parent 1959275622
commit fae9dcaed7
6 changed files with 273 additions and 173 deletions

View File

@ -44,22 +44,6 @@ public class DateMathParser {
return parse(text, now, false, null); return parse(text, now, false, null);
} }
public long parse(String text, long now, DateTimeZone timeZone) {
return parse(text, now, false, timeZone);
}
public long parseRoundCeil(String text, long now) {
return parse(text, now, true, null);
}
public long parseRoundCeil(String text, long now, DateTimeZone timeZone) {
return parse(text, now, true, timeZone);
}
public long parse(String text, long now, boolean roundCeil) {
return parse(text, now, roundCeil, null);
}
public long parse(String text, long now, boolean roundCeil, DateTimeZone timeZone) { public long parse(String text, long now, boolean roundCeil, DateTimeZone timeZone) {
long time; long time;
String mathString; String mathString;
@ -92,139 +76,110 @@ public class DateMathParser {
private long parseMath(String mathString, long time, boolean roundUp) throws ElasticsearchParseException { private long parseMath(String mathString, long time, boolean roundUp) throws ElasticsearchParseException {
MutableDateTime dateTime = new MutableDateTime(time, DateTimeZone.UTC); MutableDateTime dateTime = new MutableDateTime(time, DateTimeZone.UTC);
try { for (int i = 0; i < mathString.length(); ) {
for (int i = 0; i < mathString.length(); ) { char c = mathString.charAt(i++);
char c = mathString.charAt(i++); final boolean round;
int type; final int sign;
if (c == '/') { if (c == '/') {
type = 0; round = true;
} else if (c == '+') { sign = 1;
type = 1; } else {
round = false;
if (c == '+') {
sign = 1;
} else if (c == '-') { } else if (c == '-') {
type = 2; sign = -1;
} else { } else {
throw new ElasticsearchParseException("operator not supported for date math [" + mathString + "]"); throw new ElasticsearchParseException("operator not supported for date math [" + mathString + "]");
} }
}
if (i >= mathString.length()) {
throw new ElasticsearchParseException("truncated date math [" + mathString + "]");
}
int num; final int num;
if (!Character.isDigit(mathString.charAt(i))) { if (!Character.isDigit(mathString.charAt(i))) {
num = 1; num = 1;
} else {
int numFrom = i;
while (i < mathString.length() && Character.isDigit(mathString.charAt(i))) {
i++;
}
if (i >= mathString.length()) {
throw new ElasticsearchParseException("truncated date math [" + mathString + "]");
}
num = Integer.parseInt(mathString.substring(numFrom, i));
}
if (round) {
if (num != 1) {
throw new ElasticsearchParseException("rounding `/` can only be used on single unit types [" + mathString + "]");
}
}
char unit = mathString.charAt(i++);
MutableDateTime.Property propertyToRound = null;
switch (unit) {
case 'y':
if (round) {
propertyToRound = dateTime.yearOfCentury();
} else {
dateTime.addYears(sign * num);
}
break;
case 'M':
if (round) {
propertyToRound = dateTime.monthOfYear();
} else {
dateTime.addMonths(sign * num);
}
break;
case 'w':
if (round) {
propertyToRound = dateTime.weekOfWeekyear();
} else {
dateTime.addWeeks(sign * num);
}
break;
case 'd':
if (round) {
propertyToRound = dateTime.dayOfMonth();
} else {
dateTime.addDays(sign * num);
}
break;
case 'h':
case 'H':
if (round) {
propertyToRound = dateTime.hourOfDay();
} else {
dateTime.addHours(sign * num);
}
break;
case 'm':
if (round) {
propertyToRound = dateTime.minuteOfHour();
} else {
dateTime.addMinutes(sign * num);
}
break;
case 's':
if (round) {
propertyToRound = dateTime.secondOfMinute();
} else {
dateTime.addSeconds(sign * num);
}
break;
default:
throw new ElasticsearchParseException("unit [" + unit + "] not supported for date math [" + mathString + "]");
}
if (propertyToRound != null) {
if (roundUp) {
propertyToRound.roundCeiling();
dateTime.addMillis(-1); // subtract 1 millisecond to get the largest inclusive value
} else { } else {
int numFrom = i; propertyToRound.roundFloor();
while (Character.isDigit(mathString.charAt(i))) {
i++;
}
num = Integer.parseInt(mathString.substring(numFrom, i));
}
if (type == 0) {
// rounding is only allowed on whole numbers
if (num != 1) {
throw new ElasticsearchParseException("rounding `/` can only be used on single unit types [" + mathString + "]");
}
}
char unit = mathString.charAt(i++);
switch (unit) {
case 'y':
if (type == 0) {
if (roundUp) {
dateTime.yearOfCentury().roundCeiling();
} else {
dateTime.yearOfCentury().roundFloor();
}
} else if (type == 1) {
dateTime.addYears(num);
} else if (type == 2) {
dateTime.addYears(-num);
}
break;
case 'M':
if (type == 0) {
if (roundUp) {
dateTime.monthOfYear().roundCeiling();
} else {
dateTime.monthOfYear().roundFloor();
}
} else if (type == 1) {
dateTime.addMonths(num);
} else if (type == 2) {
dateTime.addMonths(-num);
}
break;
case 'w':
if (type == 0) {
if (roundUp) {
dateTime.weekOfWeekyear().roundCeiling();
} else {
dateTime.weekOfWeekyear().roundFloor();
}
} else if (type == 1) {
dateTime.addWeeks(num);
} else if (type == 2) {
dateTime.addWeeks(-num);
}
break;
case 'd':
if (type == 0) {
if (roundUp) {
dateTime.dayOfMonth().roundCeiling();
} else {
dateTime.dayOfMonth().roundFloor();
}
} else if (type == 1) {
dateTime.addDays(num);
} else if (type == 2) {
dateTime.addDays(-num);
}
break;
case 'h':
case 'H':
if (type == 0) {
if (roundUp) {
dateTime.hourOfDay().roundCeiling();
} else {
dateTime.hourOfDay().roundFloor();
}
} else if (type == 1) {
dateTime.addHours(num);
} else if (type == 2) {
dateTime.addHours(-num);
}
break;
case 'm':
if (type == 0) {
if (roundUp) {
dateTime.minuteOfHour().roundCeiling();
} else {
dateTime.minuteOfHour().roundFloor();
}
} else if (type == 1) {
dateTime.addMinutes(num);
} else if (type == 2) {
dateTime.addMinutes(-num);
}
break;
case 's':
if (type == 0) {
if (roundUp) {
dateTime.secondOfMinute().roundCeiling();
} else {
dateTime.secondOfMinute().roundFloor();
}
} else if (type == 1) {
dateTime.addSeconds(num);
} else if (type == 2) {
dateTime.addSeconds(-num);
}
break;
default:
throw new ElasticsearchParseException("unit [" + unit + "] not supported for date math [" + mathString + "]");
} }
} }
} catch (Exception e) {
if (e instanceof ElasticsearchParseException) {
throw (ElasticsearchParseException) e;
}
throw new ElasticsearchParseException("failed to parse date math [" + mathString + "]");
} }
return dateTime.getMillis(); return dateTime.getMillis();
} }

View File

@ -314,21 +314,22 @@ public class DateFieldMapper extends NumberFieldMapper<Long> {
return parseToMilliseconds(value, false, null, dateMathParser); return parseToMilliseconds(value, false, null, dateMathParser);
} }
public long parseToMilliseconds(Object value, boolean includeUpper, @Nullable DateTimeZone zone, @Nullable DateMathParser forcedDateParser) { public long parseToMilliseconds(Object value, boolean inclusive, @Nullable DateTimeZone zone, @Nullable DateMathParser forcedDateParser) {
if (value instanceof Number) { if (value instanceof Number) {
return ((Number) value).longValue(); return ((Number) value).longValue();
} }
return parseToMilliseconds(convertToString(value), includeUpper, zone, forcedDateParser); return parseToMilliseconds(convertToString(value), inclusive, zone, forcedDateParser);
} }
public long parseToMilliseconds(String value, boolean includeUpper, @Nullable DateTimeZone zone, @Nullable DateMathParser forcedDateParser) { public long parseToMilliseconds(String value, boolean inclusive, @Nullable DateTimeZone zone, @Nullable DateMathParser forcedDateParser) {
SearchContext sc = SearchContext.current(); SearchContext sc = SearchContext.current();
long now = sc == null ? System.currentTimeMillis() : sc.nowInMillis(); long now = sc == null ? System.currentTimeMillis() : sc.nowInMillis();
DateMathParser dateParser = dateMathParser; DateMathParser dateParser = dateMathParser;
if (forcedDateParser != null) { if (forcedDateParser != null) {
dateParser = forcedDateParser; dateParser = forcedDateParser;
} }
return includeUpper && roundCeil ? dateParser.parseRoundCeil(value, now, zone) : dateParser.parse(value, now, zone); boolean roundUp = inclusive && roundCeil;
return dateParser.parse(value, now, roundUp, zone);
} }
@Override @Override
@ -354,8 +355,13 @@ public class DateFieldMapper extends NumberFieldMapper<Long> {
private Query innerRangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser) { private Query innerRangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser) {
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep, return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,
<<<<<<< HEAD
lowerTerm == null ? null : parseToMilliseconds(lowerTerm, false, timeZone, forcedDateParser == null ? dateMathParser : forcedDateParser), lowerTerm == null ? null : parseToMilliseconds(lowerTerm, false, timeZone, forcedDateParser == null ? dateMathParser : forcedDateParser),
upperTerm == null ? null : parseToMilliseconds(upperTerm, includeUpper, timeZone, forcedDateParser == null ? dateMathParser : forcedDateParser), upperTerm == null ? null : parseToMilliseconds(upperTerm, includeUpper, timeZone, forcedDateParser == null ? dateMathParser : forcedDateParser),
=======
lowerTerm == null ? null : parseToMilliseconds(lowerTerm, context, !includeLower, timeZone, forcedDateParser == null ? dateMathParser : forcedDateParser),
upperTerm == null ? null : parseToMilliseconds(upperTerm, context, includeUpper, timeZone, forcedDateParser == null ? dateMathParser : forcedDateParser),
>>>>>>> DateMath: Fix semantics of rounding with inclusive/exclusive ranges.
includeLower, includeUpper); includeLower, includeUpper);
} }

View File

@ -19,48 +19,141 @@
package org.elasticsearch.common.joda; package org.elasticsearch.common.joda;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Test; import org.joda.time.DateTimeZone;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
/**
*/
public class DateMathParserTests extends ElasticsearchTestCase { public class DateMathParserTests extends ElasticsearchTestCase {
FormatDateTimeFormatter formatter = Joda.forPattern("dateOptionalTime");
DateMathParser parser = new DateMathParser(formatter, TimeUnit.MILLISECONDS);
@Test void assertDateMathEquals(String toTest, String expected) {
public void dataMathTests() { assertDateMathEquals(toTest, expected, 0, false, null);
}
void assertDateMathEquals(String toTest, String expected, long now, boolean roundUp, DateTimeZone timeZone) {
DateMathParser parser = new DateMathParser(Joda.forPattern("dateOptionalTime"), TimeUnit.MILLISECONDS); DateMathParser parser = new DateMathParser(Joda.forPattern("dateOptionalTime"), TimeUnit.MILLISECONDS);
long gotMillis = parser.parse(toTest, now, roundUp, null);
long expectedMillis = parser.parse(expected, 0);
if (gotMillis != expectedMillis) {
fail("Date math not equal\n" +
"Original : " + toTest + "\n" +
"Parsed : " + formatter.printer().print(gotMillis) + "\n" +
"Expected : " + expected + "\n" +
"Expected milliseconds : " + expectedMillis + "\n" +
"Actual milliseconds : " + gotMillis + "\n");
}
}
public void testBasicDates() {
assertDateMathEquals("2014", "2014-01-01T00:00:00.000");
assertDateMathEquals("2014-05", "2014-05-01T00:00:00.000");
assertDateMathEquals("2014-05-30", "2014-05-30T00:00:00.000");
assertDateMathEquals("2014-05-30T20", "2014-05-30T20:00:00.000");
assertDateMathEquals("2014-05-30T20:21", "2014-05-30T20:21:00.000");
assertDateMathEquals("2014-05-30T20:21:35", "2014-05-30T20:21:35.000");
assertDateMathEquals("2014-05-30T20:21:35.123", "2014-05-30T20:21:35.123");
}
public void testBasicMath() {
assertDateMathEquals("2014-11-18||+y", "2015-11-18");
assertDateMathEquals("2014-11-18||-2y", "2012-11-18");
assertThat(parser.parse("now", 0), equalTo(0l)); assertDateMathEquals("2014-11-18||+3M", "2015-02-18");
assertThat(parser.parse("now+m", 0), equalTo(TimeUnit.MINUTES.toMillis(1))); assertDateMathEquals("2014-11-18||-M", "2014-10-18");
assertThat(parser.parse("now+1m", 0), equalTo(TimeUnit.MINUTES.toMillis(1)));
assertThat(parser.parse("now+11m", 0), equalTo(TimeUnit.MINUTES.toMillis(11)));
assertThat(parser.parse("now+1d", 0), equalTo(TimeUnit.DAYS.toMillis(1))); assertDateMathEquals("2014-11-18||+1w", "2014-11-25");
assertDateMathEquals("2014-11-18||-3w", "2014-10-28");
assertThat(parser.parse("now+1m+1s", 0), equalTo(TimeUnit.MINUTES.toMillis(1) + TimeUnit.SECONDS.toMillis(1))); assertDateMathEquals("2014-11-18||+22d", "2014-12-10");
assertThat(parser.parse("now+1m-1s", 0), equalTo(TimeUnit.MINUTES.toMillis(1) - TimeUnit.SECONDS.toMillis(1))); assertDateMathEquals("2014-11-18||-423d", "2013-09-21");
assertThat(parser.parse("now+1m+1s/m", 0), equalTo(TimeUnit.MINUTES.toMillis(1))); assertDateMathEquals("2014-11-18T14||+13h", "2014-11-19T03");
assertThat(parser.parseRoundCeil("now+1m+1s/m", 0), equalTo(TimeUnit.MINUTES.toMillis(2))); assertDateMathEquals("2014-11-18T14||-1h", "2014-11-18T13");
assertDateMathEquals("2014-11-18T14||+13H", "2014-11-19T03");
assertThat(parser.parse("now+4y", 0), equalTo(TimeUnit.DAYS.toMillis(4*365 + 1))); assertDateMathEquals("2014-11-18T14||-1H", "2014-11-18T13");
assertDateMathEquals("2014-11-18T14:27||+10240m", "2014-11-25T17:07");
assertDateMathEquals("2014-11-18T14:27||-10m", "2014-11-18T14:17");
assertDateMathEquals("2014-11-18T14:27:32||+60s", "2014-11-18T14:28:32");
assertDateMathEquals("2014-11-18T14:27:32||-3600s", "2014-11-18T13:27:32");
} }
@Test public void testMultipleAdjustments() {
public void actualDateTests() { assertDateMathEquals("2014-11-18||+1M-1M", "2014-11-18");
DateMathParser parser = new DateMathParser(Joda.forPattern("dateOptionalTime"), TimeUnit.MILLISECONDS); assertDateMathEquals("2014-11-18||+1M-1m", "2014-12-17T23:59");
assertDateMathEquals("2014-11-18||-1m+1M", "2014-12-17T23:59");
assertDateMathEquals("2014-11-18||+1M/M", "2014-12-01");
assertDateMathEquals("2014-11-18||+1M/M+1h", "2014-12-01T01");
}
assertThat(parser.parse("1970-01-01", 0), equalTo(0l));
assertThat(parser.parse("1970-01-01||+1m", 0), equalTo(TimeUnit.MINUTES.toMillis(1))); public void testNow() {
assertThat(parser.parse("1970-01-01||+1m+1s", 0), equalTo(TimeUnit.MINUTES.toMillis(1) + TimeUnit.SECONDS.toMillis(1))); long now = parser.parse("2014-11-18T14:27:32", 0, false, null);
assertDateMathEquals("now", "2014-11-18T14:27:32", now, false, null);
assertDateMathEquals("now+M", "2014-12-18T14:27:32", now, false, null);
assertDateMathEquals("now-2d", "2014-11-16T14:27:32", now, false, null);
assertDateMathEquals("now/m", "2014-11-18T14:27", now, false, null);
}
public void testRounding() {
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);
assertDateMathEquals("2014||/y", "2014-12-31T23:59:59.999", 0, true, null);
assertThat(parser.parse("2013-01-01||+1y", 0), equalTo(parser.parse("2013-01-01", 0) + TimeUnit.DAYS.toMillis(365))); assertDateMathEquals("2014-11-18||/M", "2014-11-01", 0, false, null);
assertThat(parser.parse("2013-03-03||/y", 0), equalTo(parser.parse("2013-01-01", 0))); assertDateMathEquals("2014-11-18||/M", "2014-11-30T23:59:59.999", 0, true, null);
assertThat(parser.parseRoundCeil("2013-03-03||/y", 0), equalTo(parser.parse("2014-01-01", 0))); assertDateMathEquals("2014-11||/M", "2014-11-01", 0, false, null);
assertDateMathEquals("2014-11||/M", "2014-11-30T23:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14||/w", "2014-11-17", 0, false, null);
assertDateMathEquals("2014-11-18T14||/w", "2014-11-23T23:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18||/w", "2014-11-17", 0, false, null);
assertDateMathEquals("2014-11-18||/w", "2014-11-23T23:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14||/d", "2014-11-18", 0, false, null);
assertDateMathEquals("2014-11-18T14||/d", "2014-11-18T23:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18||/d", "2014-11-18", 0, false, null);
assertDateMathEquals("2014-11-18||/d", "2014-11-18T23:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14:27||/h", "2014-11-18T14", 0, false, null);
assertDateMathEquals("2014-11-18T14:27||/h", "2014-11-18T14:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14||/H", "2014-11-18T14", 0, false, null);
assertDateMathEquals("2014-11-18T14||/H", "2014-11-18T14:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14:27||/h", "2014-11-18T14", 0, false, null);
assertDateMathEquals("2014-11-18T14:27||/h", "2014-11-18T14:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14||/H", "2014-11-18T14", 0, false, null);
assertDateMathEquals("2014-11-18T14||/H", "2014-11-18T14:59:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14:27:32||/m", "2014-11-18T14:27", 0, false, null);
assertDateMathEquals("2014-11-18T14:27:32||/m", "2014-11-18T14:27:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14:27||/m", "2014-11-18T14:27", 0, false, null);
assertDateMathEquals("2014-11-18T14:27||/m", "2014-11-18T14:27:59.999", 0, true, null);
assertDateMathEquals("2014-11-18T14:27:32.123||/s", "2014-11-18T14:27:32", 0, false, null);
assertDateMathEquals("2014-11-18T14:27:32.123||/s", "2014-11-18T14:27:32.999", 0, true, null);
assertDateMathEquals("2014-11-18T14:27:32||/s", "2014-11-18T14:27:32", 0, false, null);
assertDateMathEquals("2014-11-18T14:27:32||/s", "2014-11-18T14:27:32.999", 0, true, null);
}
void assertParseException(String msg, String date) {
try {
parser.parse(date, 0);
fail("Date: " + date + "\n" + msg);
} catch (ElasticsearchParseException e) {
// expected
}
}
public void testIllegalMathFormat() {
assertParseException("Expected date math unsupported operator exception", "2014-11-18||*5");
assertParseException("Expected date math incompatible rounding exception", "2014-11-18||/2m");
assertParseException("Expected date math illegal unit type exception", "2014-11-18||+2a");
assertParseException("Expected date math truncation exception", "2014-11-18||+12");
assertParseException("Expected date math truncation exception", "2014-11-18||-");
} }
} }

View File

@ -120,4 +120,34 @@ public class IndexQueryParserFilterDateRangeFormatTests extends ElasticsearchSin
SearchContext.removeCurrent(); SearchContext.removeCurrent();
} }
} }
@Test
public void testDateRangeBoundaries() throws IOException {
IndexQueryParserService queryParser = queryParser();
String query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_query_boundaries_inclusive.json");
Query parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class));
NumericRangeQuery rangeQuery = (NumericRangeQuery) parsedQuery;
DateTime min = DateTime.parse("2014-11-01T00:00:00.000+00");
assertThat(rangeQuery.getMin().longValue(), is(min.getMillis()));
assertTrue(rangeQuery.includesMin());
DateTime max = DateTime.parse("2014-12-08T23:59:59.999+00");
assertThat(rangeQuery.getMax().longValue(), is(max.getMillis()));
assertTrue(rangeQuery.includesMax());
query = copyToStringFromClasspath("/org/elasticsearch/index/query/date_range_query_boundaries_exclusive.json");
parsedQuery = queryParser.parse(query).query();
assertThat(parsedQuery, instanceOf(NumericRangeQuery.class));
rangeQuery = (NumericRangeQuery) parsedQuery;
min = DateTime.parse("2014-11-30T23:59:59.999+00");
assertThat(rangeQuery.getMin().longValue(), is(min.getMillis()));
assertFalse(rangeQuery.includesMin());
max = DateTime.parse("2014-12-08T00:00:00.000+00");
assertThat(rangeQuery.getMax().longValue(), is(max.getMillis()));
assertFalse(rangeQuery.includesMax());
}
} }

View File

@ -0,0 +1,8 @@
{
"range" : {
"born" : {
"gt": "2014-11-05||/M",
"lt": "2014-12-08||/d"
}
}
}

View File

@ -0,0 +1,8 @@
{
"range" : {
"born" : {
"gte": "2014-11-05||/M",
"lte": "2014-12-08||/d"
}
}
}