Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a DST error in date_histogram #52016

Merged
merged 6 commits into from
Feb 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ setup:
settings:
number_of_replicas: 0
mappings:
"properties":
"number":
"type" : "integer"
"date":
"type" : "date"
properties:
number:
type: integer
date:
type: date
fields:
nanos:
type: date_nanos
- do:
cluster.health:
wait_for_status: green
Expand Down Expand Up @@ -156,7 +159,10 @@ setup:
mappings:
properties:
date:
type : date
type: date
fields:
nanos:
type: date_nanos

- do:
bulk:
Expand All @@ -181,7 +187,24 @@ setup:
date_histogram:
field: date
calendar_interval: month
- match: { hits.total.value: 4 }
- length: { aggregations.histo.buckets: 3 }
- match: { aggregations.histo.buckets.0.key_as_string: "2016-01-01T00:00:00.000Z" }
- match: { aggregations.histo.buckets.0.doc_count: 2 }
- match: { aggregations.histo.buckets.1.key_as_string: "2016-02-01T00:00:00.000Z" }
- match: { aggregations.histo.buckets.1.doc_count: 1 }
- match: { aggregations.histo.buckets.2.key_as_string: "2016-03-01T00:00:00.000Z" }
- match: { aggregations.histo.buckets.2.doc_count: 1 }

- do:
search:
body:
size: 0
aggs:
histo:
date_histogram:
field: date.nanos
calendar_interval: month
- match: { hits.total.value: 4 }
- length: { aggregations.histo.buckets: 3 }
- match: { aggregations.histo.buckets.0.key_as_string: "2016-01-01T00:00:00.000Z" }
Expand Down Expand Up @@ -349,3 +372,49 @@ setup:
- match: { aggregations.histo.buckets.1.doc_count: 2 }
- match: { aggregations.histo.buckets.2.key_as_string: "2016-02-02T00:00:00.000Z" }
- match: { aggregations.histo.buckets.2.doc_count: 1 }

---
"date_histogram with pre-epoch daylight savings time transition":
- skip:
version: " - 7.99.99"
reason: bug fixed in 8.0.0. will be backported to 7.6.1

- do:
bulk:
index: test_1
refresh: true
body:
- '{"index": {}}'
- '{"date": "2016-01-01"}'

- do:
search:
body:
size: 0
aggs:
histo:
date_histogram:
field: date
fixed_interval: 1ms
time_zone: America/Phoenix

- match: { hits.total.value: 1 }
- length: { aggregations.histo.buckets: 1 }
- match: { aggregations.histo.buckets.0.key_as_string: "2015-12-31T17:00:00.000-07:00" }
- match: { aggregations.histo.buckets.0.doc_count: 1 }

- do:
search:
body:
size: 0
aggs:
histo:
date_histogram:
field: date.nanos
fixed_interval: 1ms
time_zone: America/Phoenix

- match: { hits.total.value: 1 }
- length: { aggregations.histo.buckets: 1 }
- match: { aggregations.histo.buckets.0.key_as_string: "2015-12-31T17:00:00.000-07:00" }
- match: { aggregations.histo.buckets.0.doc_count: 1 }
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public static ZoneId of(String zoneId) {
return ZoneId.of(zoneId).normalized();
}

private static final Instant MAX_NANOSECOND_INSTANT = Instant.parse("2262-04-11T23:47:16.854775807Z");
static final Instant MAX_NANOSECOND_INSTANT = Instant.parse("2262-04-11T23:47:16.854775807Z");

static final long MAX_NANOSECOND_IN_MILLIS = MAX_NANOSECOND_INSTANT.toEpochMilli();

Expand All @@ -228,6 +228,26 @@ public static long toLong(Instant instant) {
return instant.getEpochSecond() * 1_000_000_000 + instant.getNano();
}

/**
* Returns an instant that is with valid nanosecond resolution. If
* the parameter is before the valid nanosecond range then this returns
* the minimum {@linkplain Instant} valid for nanosecond resultion. If
* the parameter is after the valid nanosecond range then this returns
* the maximum {@linkplain Instant} valid for nanosecond resolution.
* <p>
* Useful for checking if all values for the field are within some range,
* even if the range's endpoints are not valid nanosecond resolution.
*/
public static Instant clampToNanosRange(Instant instant) {
if (instant.isBefore(Instant.EPOCH)) {
return Instant.EPOCH;
}
if (instant.isAfter(MAX_NANOSECOND_INSTANT)) {
return MAX_NANOSECOND_INSTANT;
}
return instant;
}

/**
* convert a long value to a java time instant
* the long value resembles the nanoseconds since the epoch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@ public long convert(Instant instant) {
public Instant toInstant(long value) {
return Instant.ofEpochMilli(value);
}

@Override
public Instant clampToValidRange(Instant instant) {
return instant;
}
},
NANOSECONDS("date_nanos", NumericType.DATE_NANOSECONDS) {
@Override
Expand All @@ -99,6 +104,11 @@ public long convert(Instant instant) {
public Instant toInstant(long value) {
return DateUtils.toInstant(value);
}

@Override
public Instant clampToValidRange(Instant instant) {
return DateUtils.clampToNanosRange(instant);
}
};

private final String type;
Expand All @@ -117,10 +127,18 @@ NumericType numericType() {
return numericType;
}

/**
* Convert an {@linkplain Instant} into a long value in this resolution.
*/
public abstract long convert(Instant instant);

/**
* Convert a long value in this resolution into an instant.
*/
public abstract Instant toInstant(long value);

public abstract Instant clampToValidRange(Instant instant);

public static Resolution ofOrdinal(int ord) {
for (Resolution resolution : values()) {
if (ord == resolution.ordinal()) {
Expand Down Expand Up @@ -440,9 +458,30 @@ public Relation isFieldWithinQuery(IndexReader reader,
}
}

// This check needs to be done after fromInclusive and toInclusive
// are resolved so we can throw an exception if they are invalid
// even if there are no points in the shard
return isFieldWithinRange(reader, fromInclusive, toInclusive);
}

/**
* Return whether all values of the given {@link IndexReader} are within the range,
* outside the range or cross the range. Unlike {@link #isFieldWithinQuery} this
* accepts values that are out of the range of the {@link #resolution} of this field.
* @param fromInclusive start date, inclusive
* @param toInclusive end date, inclusive
*/
public Relation isFieldWithinRange(IndexReader reader, Instant fromInclusive, Instant toInclusive)
throws IOException {
return isFieldWithinRange(reader,
resolution.convert(resolution.clampToValidRange(fromInclusive)),
resolution.convert(resolution.clampToValidRange(toInclusive)));
}

/**
* Return whether all values of the given {@link IndexReader} are within the range,
* outside the range or cross the range.
* @param fromInclusive start date, inclusive, {@link Resolution#convert(Instant) converted} to the appropriate scale
* @param toInclusive end date, inclusive, {@link Resolution#convert(Instant) converted} to the appropriate scale
*/
private Relation isFieldWithinRange(IndexReader reader, long fromInclusive, long toInclusive) throws IOException {
if (PointValues.size(reader, name()) == 0) {
// no points, so nothing matches
return Relation.DISJOINT;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,10 @@ public enum Relation {
* {@link Relation#INTERSECTS}, which is always fine to return when there is
* no way to check whether values are actually within bounds. */
public Relation isFieldWithinQuery(
IndexReader reader,
Object from, Object to,
boolean includeLower, boolean includeUpper,
ZoneId timeZone, DateMathParser dateMathParser, QueryRewriteContext context) throws IOException {
IndexReader reader,
Object from, Object to,
boolean includeLower, boolean includeUpper,
ZoneId timeZone, DateMathParser dateMathParser, QueryRewriteContext context) throws IOException {
return Relation.INTERSECTS;
}

Expand Down
Loading