From 5e0ced5c14e6d971d870dbbce2e3e744266efdaf Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 11 Feb 2020 16:55:14 -0500 Subject: [PATCH] Support time_zone on composite's date_histogram (#51172) (#51494) 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 --- .../test/search.aggregation/230_composite.yml | 68 ++++++++++ .../CompositeValuesSourceBuilder.java | 13 +- .../DateHistogramValuesSourceBuilder.java | 1 + .../composite/CompositeAggregatorTests.java | 117 ++++++++++-------- 4 files changed, 142 insertions(+), 57 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml index 7aa495fb0c321..6787629789ae6 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml @@ -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 } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java index bf7007a654485..96f95eb171d2a 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeValuesSourceBuilder.java @@ -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; /** @@ -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; + } } diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java index 1e1f843406461..2c76872479074 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/DateHistogramValuesSourceBuilder.java @@ -228,6 +228,7 @@ public DateHistogramValuesSourceBuilder timeZone(ZoneId timeZone) { /** * Gets the time zone to use for this aggregation */ + @Override public ZoneId timeZone() { return timeZone; } diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java index 7769dee3c5833..7685cd9067ddc 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregatorTests.java @@ -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; @@ -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 { @@ -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>> 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>> dataset = new ArrayList<>(); dataset.addAll(