Fix round up of date range without rounding (#43303)

Today when searching for an exclusive range the java date math parser rounds up the value
with the granularity of the operation. So when searching for values that are greater than
"now-2M" the parser rounds up the operation to "now-1M". This behavior was introduced when
we migrated to java date but it looks like a bug since the joda math parser rounds up values
but only when a rounding is used. So "now/M" is rounded to "now-1ms" (minus 1ms to get the largest inclusive value)
in the joda parser if the result should be exclusive but no rounding is applied if the input
is a simple operation like "now-1M". This change restores the joda behavior in order to have
a consistent parsing in all versions.

Closes #43277
This commit is contained in:
Jim Ferenczi 2019-06-20 22:01:18 +02:00 committed by jimczi
parent 827f8fcbd5
commit cc6c114cb8
4 changed files with 46 additions and 35 deletions

View File

@ -31,7 +31,7 @@ import java.util.function.LongSupplier;
public interface DateMathParser {
/**
* Parse a date math expression without timzeone info and rounding down.
* Parse a date math expression without timezone info and rounding down.
*/
default Instant parse(String text, LongSupplier now) {
return parse(text, now, false, (ZoneId) null);
@ -43,8 +43,8 @@ public interface DateMathParser {
// exists for backcompat, do not use!
@Deprecated
default Instant parse(String text, LongSupplier now, boolean roundUp, DateTimeZone tz) {
return parse(text, now, roundUp, tz == null ? null : ZoneId.of(tz.getID()));
default Instant parse(String text, LongSupplier now, boolean roundUpProperty, DateTimeZone tz) {
return parse(text, now, roundUpProperty, tz == null ? null : ZoneId.of(tz.getID()));
}
/**
@ -65,11 +65,11 @@ public interface DateMathParser {
* s second
*
*
* @param text the input
* @param now a supplier to retrieve the current date in milliseconds, if needed for additions
* @param roundUp should the result be rounded up
* @param tz an optional timezone that should be applied before returning the milliseconds since the epoch
* @return the parsed date as an Instant since the epoch
* @param text the input
* @param now a supplier to retrieve the current date in milliseconds, if needed for additions
* @param roundUpProperty should the result be rounded up with the granularity of the rounding (e.g. <code>now/M</code>)
* @param tz an optional timezone that should be applied before returning the milliseconds since the epoch
* @return the parsed date as an Instant since the epoch
*/
Instant parse(String text, LongSupplier now, boolean roundUp, ZoneId tz);
Instant parse(String text, LongSupplier now, boolean roundUpProperty, ZoneId tz);
}

View File

@ -59,7 +59,7 @@ public class JavaDateMathParser implements DateMathParser {
}
@Override
public Instant parse(String text, LongSupplier now, boolean roundUp, ZoneId timeZone) {
public Instant parse(String text, LongSupplier now, boolean roundUpProperty, ZoneId timeZone) {
Instant time;
String mathString;
if (text.startsWith("now")) {
@ -73,16 +73,16 @@ public class JavaDateMathParser implements DateMathParser {
} else {
int index = text.indexOf("||");
if (index == -1) {
return parseDateTime(text, timeZone, roundUp);
return parseDateTime(text, timeZone, roundUpProperty);
}
time = parseDateTime(text.substring(0, index), timeZone, false);
mathString = text.substring(index + 2);
}
return parseMath(mathString, time, roundUp, timeZone);
return parseMath(mathString, time, roundUpProperty, timeZone);
}
private Instant parseMath(final String mathString, final Instant time, final boolean roundUp,
private Instant parseMath(final String mathString, final Instant time, final boolean roundUpProperty,
ZoneId timeZone) throws ElasticsearchParseException {
if (timeZone == null) {
timeZone = ZoneOffset.UTC;
@ -133,78 +133,79 @@ public class JavaDateMathParser implements DateMathParser {
case 'y':
if (round) {
dateTime = dateTime.withDayOfYear(1).with(LocalTime.MIN);
if (roundUpProperty) {
dateTime = dateTime.plusYears(1);
}
} else {
dateTime = dateTime.plusYears(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusYears(1);
}
break;
case 'M':
if (round) {
dateTime = dateTime.withDayOfMonth(1).with(LocalTime.MIN);
if (roundUpProperty) {
dateTime = dateTime.plusMonths(1);
}
} else {
dateTime = dateTime.plusMonths(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusMonths(1);
}
break;
case 'w':
if (round) {
dateTime = dateTime.with(TemporalAdjusters.previousOrSame(DayOfWeek.MONDAY)).with(LocalTime.MIN);
if (roundUpProperty) {
dateTime = dateTime.plusWeeks(1);
}
} else {
dateTime = dateTime.plusWeeks(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusWeeks(1);
}
break;
case 'd':
if (round) {
dateTime = dateTime.with(LocalTime.MIN);
if (roundUpProperty) {
dateTime = dateTime.plusDays(1);
}
} else {
dateTime = dateTime.plusDays(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusDays(1);
}
break;
case 'h':
case 'H':
if (round) {
dateTime = dateTime.withMinute(0).withSecond(0).withNano(0);
if (roundUpProperty) {
dateTime = dateTime.plusHours(1);
}
} else {
dateTime = dateTime.plusHours(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusHours(1);
}
break;
case 'm':
if (round) {
dateTime = dateTime.withSecond(0).withNano(0);
if (roundUpProperty) {
dateTime = dateTime.plusMinutes(1);
}
} else {
dateTime = dateTime.plusMinutes(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusMinutes(1);
}
break;
case 's':
if (round) {
dateTime = dateTime.withNano(0);
if (roundUpProperty) {
dateTime = dateTime.plusSeconds(1);
}
} else {
dateTime = dateTime.plusSeconds(sign * num);
}
if (roundUp) {
dateTime = dateTime.plusSeconds(1);
}
break;
default:
throw new ElasticsearchParseException("unit [{}] not supported for date math [{}]", unit, mathString);
}
if (roundUp) {
if (round && roundUpProperty) {
// subtract 1 millisecond to get the largest inclusive value
dateTime = dateTime.minus(1, ChronoField.MILLI_OF_SECOND.getBaseUnit());
}
}

View File

@ -151,8 +151,13 @@ public class JodaDateMathParserTests extends ESTestCase {
assertDateMathEquals("now", "2014-11-18T14:27:32", now, false, null);
assertDateMathEquals("now+M", "2014-12-18T14:27:32", now, false, null);
assertDateMathEquals("now+M", "2014-12-18T14:27:32", now, true, null);
assertDateMathEquals("now-2d", "2014-11-16T14:27:32", now, false, null);
assertDateMathEquals("now-2d", "2014-11-16T14:27:32", now, true, null);
assertDateMathEquals("now/m", "2014-11-18T14:27", now, false, null);
assertDateMathEquals("now/m", "2014-11-18T14:27:59.999Z", now, true, null);
assertDateMathEquals("now/M", "2014-11-01T00:00:00", now, false, null);
assertDateMathEquals("now/M", "2014-11-30T23:59:59.999Z", now, true, null);
// timezone does not affect now
assertDateMathEquals("now/m", "2014-11-18T14:27", now, false, DateTimeZone.forID("+02:00"));

View File

@ -139,8 +139,13 @@ public class JavaDateMathParserTests extends ESTestCase {
assertDateMathEquals("now", "2014-11-18T14:27:32", now, false, null);
assertDateMathEquals("now+M", "2014-12-18T14:27:32", now, false, null);
assertDateMathEquals("now+M", "2014-12-18T14:27:32", now, true, null);
assertDateMathEquals("now-2d", "2014-11-16T14:27:32", now, false, null);
assertDateMathEquals("now-2d", "2014-11-16T14:27:32", now, true, null);
assertDateMathEquals("now/m", "2014-11-18T14:27", now, false, null);
assertDateMathEquals("now/m", "2014-11-18T14:27:59.999Z", now, true, null);
assertDateMathEquals("now/M", "2014-11-01T00:00:00", now, false, null);
assertDateMathEquals("now/M", "2014-11-30T23:59:59.999Z", now, true, null);
// timezone does not affect now
assertDateMathEquals("now/m", "2014-11-18T14:27", now, false, ZoneId.of("+02:00"));