Skip to content

Commit

Permalink
Support time_zone on composite's date_histogram (#51172) (#51491)
Browse files Browse the repository at this point in the history 
We've been parsing the `time_zone` parameter on `date_hitogram` for a
while but it hasn't *done* anything. This wires it up.

Closes #45199
Inspired by #45200
  • Loading branch information
@nik9000
nik9000 authored Jan 27, 2020
1 parent 3c223ce commit faf6bb2
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 57 deletions.
68 changes: 68 additions & 0 deletions rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -817,3 +817,71 @@ setup:
- length: { aggregations.test.buckets: 1 }
- match: { aggregations.test.buckets.0.key.f: "192.168.0.1" }
- match: { aggregations.test.buckets.0.doc_count: 1 }

---
"date_histogram with time_zone":
- skip:
version: " - 7.99.99"
reason: This will fail against 7.whatever until we backport the fix
- do:
index:
index: test
id: 7
body: { "date": "2017-10-22T01:00:00" }
refresh: true
- do:
search:
index: test
body:
aggregations:
test:
composite:
sources: [
{
"date": {
"date_histogram": {
"field": "date",
"calendar_interval": "1d",
"time_zone": "-02:00",
"format": "iso8601" # Format makes the comparisons a little more obvious
}
}
}
]

- match: { hits.total.value: 7 }
- match: { hits.total.relation: eq }
- length: { aggregations.test.buckets: 2 }
- match: { aggregations.test.buckets.0.key.date: "2017-10-20T00:00:00.000-02:00" }
- match: { aggregations.test.buckets.0.doc_count: 1 }
- match: { aggregations.test.buckets.1.key.date: "2017-10-21T00:00:00.000-02:00" }
- match: { aggregations.test.buckets.1.doc_count: 2 }

- do:
search:
index: test
body:
aggregations:
test:
composite:
after: {
date: "2017-10-20"
}
sources: [
{
"date": {
"date_histogram": {
"field": "date",
"calendar_interval": "1d",
"time_zone": "-02:00",
"format": "iso8601" # Format makes the comparisons a little more obvious
}
}
}
]

- match: { hits.total.value: 7 }
- match: { hits.total.relation: eq }
- length: { aggregations.test.buckets: 1 }
- match: { aggregations.test.buckets.0.key.date: "2017-10-21T00:00:00.000-02:00" }
- match: { aggregations.test.buckets.0.doc_count: 2 }
13 changes: 11 additions & 2 deletions .../org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@
import org.elasticsearch.index.query.QueryShardContext;
import org.elasticsearch.script.Script;
import org.elasticsearch.search.aggregations.support.ValueType;
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
import org.elasticsearch.search.sort.SortOrder;

import java.io.IOException;
import java.time.ZoneId;
import java.util.Objects;

/**
Expand Down Expand Up @@ -297,7 +298,15 @@ protected abstract CompositeValuesSourceConfig innerBuild(QueryShardContext quer

public final CompositeValuesSourceConfig build(QueryShardContext queryShardContext) throws IOException {
ValuesSourceConfig<?> config = ValuesSourceConfig.resolve(queryShardContext,
valueType, field, script, null,null, format);
valueType, field, script, null, timeZone(), format);
return innerBuild(queryShardContext, config);
}

/**
* The time zone for this value source. Default implementation returns {@code null}
* because most value source types don't support time zone.
*/
protected ZoneId timeZone() {
return null;
}
}
1 change: 1 addition & 0 deletions .../elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ public DateHistogramValuesSourceBuilder timeZone(ZoneId timeZone) {
/**
* Gets the time zone to use for this aggregation
*/
@Override
public ZoneId timeZone() {
return timeZone;
}
Expand Down
117 changes: 62 additions & 55 deletions ...java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@

import java.io.IOException;
import java.net.InetAddress;
import java.time.ZoneOffset;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -1088,6 +1088,67 @@ public void testWithDateHistogram() throws IOException {
assertEquals(1L, result.getBuckets().get(2).getDocCount());
}
);

/*
* Tests the -04:00 time zone. This functions identically to
* the four hour offset.
*/
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("date"),
LongPoint.newRangeQuery(
"date",
asLong("2016-09-20T09:00:34"),
asLong("2017-10-20T06:09:24")
)), dataset,
() -> {
DateHistogramValuesSourceBuilder histo = new DateHistogramValuesSourceBuilder("date")
.field("date")
.calendarInterval(DateHistogramInterval.days(1))
.timeZone(ZoneId.of("-04:00"));
return new CompositeAggregationBuilder("name", Collections.singletonList(histo))
.aggregateAfter(createAfterKey("date", 1474329600000L));

}, (result) -> {
assertEquals(3, result.getBuckets().size());
assertEquals("{date=1508472000000}", result.afterKey().toString());
assertEquals("{date=1474344000000}", result.getBuckets().get(0).getKeyAsString());
assertEquals(2L, result.getBuckets().get(0).getDocCount());
assertEquals("{date=1508385600000}", result.getBuckets().get(1).getKeyAsString());
assertEquals(2L, result.getBuckets().get(1).getDocCount());
assertEquals("{date=1508472000000}", result.getBuckets().get(2).getKeyAsString());
assertEquals(1L, result.getBuckets().get(2).getDocCount());
}
);

/*
* Tests a four hour offset with a time zone, demonstrating
* why we support both things.
*/
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("date"),
LongPoint.newRangeQuery(
"date",
asLong("2016-09-20T09:00:34"),
asLong("2017-10-20T06:09:24")
)), dataset,
() -> {
DateHistogramValuesSourceBuilder histo = new DateHistogramValuesSourceBuilder("date")
.field("date")
.calendarInterval(DateHistogramInterval.days(1))
.offset(TimeUnit.HOURS.toMillis(4))
.timeZone(ZoneId.of("America/Los_Angeles"));
return new CompositeAggregationBuilder("name", Collections.singletonList(histo))
.aggregateAfter(createAfterKey("date", 1474329600000L));

}, (result) -> {
assertEquals(3, result.getBuckets().size());
assertEquals("{date=1508410800000}", result.afterKey().toString());
assertEquals("{date=1474369200000}", result.getBuckets().get(0).getKeyAsString());
assertEquals(1L, result.getBuckets().get(0).getDocCount());
assertEquals("{date=1508324400000}", result.getBuckets().get(1).getKeyAsString());
assertEquals(1L, result.getBuckets().get(1).getDocCount());
assertEquals("{date=1508410800000}", result.getBuckets().get(2).getKeyAsString());
assertEquals(2L, result.getBuckets().get(2).getDocCount());
}
);
}

public void testWithDateTerms() throws IOException {
Expand Down Expand Up @@ -1219,60 +1280,6 @@ public void testThatDateHistogramFailsFormatAfter() throws IOException {
assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.");
}

public void testWithDateHistogramAndTimeZone() throws IOException {
final List<Map<String, List<Object>>> dataset = new ArrayList<>();
dataset.addAll(
Arrays.asList(
createDocument("date", asLong("2017-10-20T03:08:45")),
createDocument("date", asLong("2016-09-20T09:00:34")),
createDocument("date", asLong("2016-09-20T11:34:00")),
createDocument("date", asLong("2017-10-20T06:09:24")),
createDocument("date", asLong("2017-10-19T06:09:24")),
createDocument("long", 4L)
)
);
testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("date")), dataset,
() -> {
DateHistogramValuesSourceBuilder histo = new DateHistogramValuesSourceBuilder("date")
.field("date")
.dateHistogramInterval(DateHistogramInterval.days(1))
.timeZone(ZoneOffset.ofHours(1));
return new CompositeAggregationBuilder("name", Collections.singletonList(histo));
},
(result) -> {
assertEquals(3, result.getBuckets().size());
assertEquals("{date=1508454000000}", result.afterKey().toString());
assertEquals("{date=1474326000000}", result.getBuckets().get(0).getKeyAsString());
assertEquals(2L, result.getBuckets().get(0).getDocCount());
assertEquals("{date=1508367600000}", result.getBuckets().get(1).getKeyAsString());
assertEquals(1L, result.getBuckets().get(1).getDocCount());
assertEquals("{date=1508454000000}", result.getBuckets().get(2).getKeyAsString());
assertEquals(2L, result.getBuckets().get(2).getDocCount());
}
);

testSearchCase(Arrays.asList(new MatchAllDocsQuery(), new DocValuesFieldExistsQuery("date")), dataset,
() -> {
DateHistogramValuesSourceBuilder histo = new DateHistogramValuesSourceBuilder("date")
.field("date")
.dateHistogramInterval(DateHistogramInterval.days(1))
.timeZone(ZoneOffset.ofHours(1));
return new CompositeAggregationBuilder("name", Collections.singletonList(histo))
.aggregateAfter(createAfterKey("date", 1474326000000L));

}, (result) -> {
assertEquals(2, result.getBuckets().size());
assertEquals("{date=1508454000000}", result.afterKey().toString());
assertEquals("{date=1508367600000}", result.getBuckets().get(0).getKeyAsString());
assertEquals(1L, result.getBuckets().get(0).getDocCount());
assertEquals("{date=1508454000000}", result.getBuckets().get(1).getKeyAsString());
assertEquals(2L, result.getBuckets().get(1).getDocCount());
}
);

assertWarnings("[interval] on [date_histogram] is deprecated, use [fixed_interval] or [calendar_interval] in the future.");
}

public void testWithDateHistogramAndKeyword() throws IOException {
final List<Map<String, List<Object>>> dataset = new ArrayList<>();
dataset.addAll(
Expand Down

0 comments on commit faf6bb2

Please sign in to comment.