Skip to content

Commit

Permalink
DateMath: Fix semantics of rounding with inclusive/exclusive ranges.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@rjernst
rjernst committed Nov 21, 2014
1 parent 1959275 commit fae9dca
Show file tree
Hide file tree
Showing 6 changed files with 271 additions and 171 deletions.
233 changes: 94 additions & 139 deletions src/main/java/org/elasticsearch/common/joda/DateMathParser.java
View file
Expand Up @@ -44,22 +44,6 @@ public long parse(String text, long now) {
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) {
long time;
String mathString;
Expand Down Expand Up @@ -92,139 +76,110 @@ public long parse(String text, long now, boolean roundCeil, DateTimeZone timeZon

private long parseMath(String mathString, long time, boolean roundUp) throws ElasticsearchParseException {
MutableDateTime dateTime = new MutableDateTime(time, DateTimeZone.UTC);
try {
for (int i = 0; i < mathString.length(); ) {
char c = mathString.charAt(i++);
int type;
if (c == '/') {
type = 0;
} else if (c == '+') {
type = 1;
for (int i = 0; i < mathString.length(); ) {
char c = mathString.charAt(i++);
final boolean round;
final int sign;
if (c == '/') {
round = true;
sign = 1;
} else {
round = false;
if (c == '+') {
sign = 1;
} else if (c == '-') {
type = 2;
sign = -1;
} else {
throw new ElasticsearchParseException("operator not supported for date math [" + mathString + "]");
}
}

if (i >= mathString.length()) {
throw new ElasticsearchParseException("truncated date math [" + mathString + "]");
}

int num;
if (!Character.isDigit(mathString.charAt(i))) {
num = 1;
} else {
int numFrom = i;
while (Character.isDigit(mathString.charAt(i))) {
i++;
}
num = Integer.parseInt(mathString.substring(numFrom, i));
final int num;
if (!Character.isDigit(mathString.charAt(i))) {
num = 1;
} else {
int numFrom = i;
while (i < mathString.length() && Character.isDigit(mathString.charAt(i))) {
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 + "]");
}
if (i >= mathString.length()) {
throw new ElasticsearchParseException("truncated date math [" + 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 + "]");
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 + "]");
}
}
} catch (Exception e) {
if (e instanceof ElasticsearchParseException) {
throw (ElasticsearchParseException) e;
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 {
propertyToRound.roundFloor();
}
}
throw new ElasticsearchParseException("failed to parse date math [" + mathString + "]");
}
return dateTime.getMillis();
}
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/org/elasticsearch/index/mapper/core/DateFieldMapper.java
View file
Expand Up @@ -314,21 +314,22 @@ public long parseToMilliseconds(Object value) {
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) {
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();
long now = sc == null ? System.currentTimeMillis() : sc.nowInMillis();
DateMathParser dateParser = dateMathParser;
if (forcedDateParser != null) {
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
Expand All @@ -354,8 +355,13 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower

private Query innerRangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, @Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser) {
return NumericRangeQuery.newLongRange(names.indexName(), precisionStep,
<<<<<<< HEAD
lowerTerm == null ? null : parseToMilliseconds(lowerTerm, false, 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);
}

Expand Down

0 comments on commit fae9dca

Please sign in to comment.