From a91520474dcfe0c7792e0597177bc4c88e718055 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 9 Mar 2020 16:10:45 -0400 Subject: [PATCH 1/3] Fix date_nanos in composite aggs It looks like `date_nanos` fields weren't likely to work properly in composite aggs because composites iterate field values using points and we weren't converting the points into milliseconds. Because the doc values were coming back in milliseconds we ended up geting very confused and just never collecting sub-aggregations. This fixes that by adding a method to `DateFieldMapper.Resolution` to `parsePointAsMillis` which is similarly in name and function to `NumberFieldMapper.NumberType`'s `parsePoint` except that it normalizes to milliseconds which is what aggs need at the moment. Closes #53168 --- .../test/search.aggregation/230_composite.yml | 53 +++++++++++++++++++ .../index/mapper/DateFieldMapper.java | 19 +++++++ .../bucket/composite/LongValuesSource.java | 3 +- .../aggregations/metrics/MinAggregator.java | 9 +--- 4 files changed, 76 insertions(+), 8 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 49afbb8afc714..d2daeb112e447 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 @@ -8,6 +8,8 @@ setup: properties: date: type: date + date_nanos: + type: date_nanos keyword: type: keyword long: @@ -850,3 +852,54 @@ setup: - 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 } + +--- +"date_histogram on date_nanos": + - skip: + version: " - 7.99.99" + reason: Fixed in 8.0.0 with backport pending to 7.7.0 + - do: + index: + index: test + id: 7 + body: { "date_nanos": "2017-11-21T01:00:00" } + refresh: true + - do: + index: + index: test + id: 8 + body: { "date_nanos": "2017-11-22T01:00:00" } + refresh: true + - do: + index: + index: test + id: 9 + body: { "date_nanos": "2017-11-22T02:00:00" } + refresh: true + - do: + search: + index: test + body: + aggregations: + test: + composite: + sources: + - date: + date_histogram: + field: date_nanos + calendar_interval: 1d + format: iso8601 # Format makes the comparisons a little more obvious + aggregations: + avg: + avg: + field: date_nanos + + - match: { hits.total.value: 9 } + - match: { hits.total.relation: eq } + - length: { aggregations.test.buckets: 2 } + - match: { aggregations.test.buckets.0.key.date: "2017-11-21T00:00:00.000Z" } + - match: { aggregations.test.buckets.0.doc_count: 1 } + - match: { aggregations.test.buckets.0.avg.value_as_string: "2017-11-21T01:00:00.000Z" } + - match: { aggregations.test.buckets.1.key.date: "2017-11-22T00:00:00.000Z" } + - match: { aggregations.test.buckets.1.doc_count: 2 } + - match: { aggregations.test.buckets.1.avg.value_as_string: "2017-11-22T01:30:00.000Z" } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java index 87d489d04b556..4e6b04a72418a 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java @@ -97,6 +97,11 @@ public Instant toInstant(long value) { public Instant clampToValidRange(Instant instant) { return instant; } + + @Override + public long parsePointAsMillis(byte[] value) { + return LongPoint.decodeDimension(value, 0); + } }, NANOSECONDS(DATE_NANOS_CONTENT_TYPE, NumericType.DATE_NANOSECONDS) { @Override @@ -113,6 +118,11 @@ public Instant toInstant(long value) { public Instant clampToValidRange(Instant instant) { return DateUtils.clampToNanosRange(instant); } + + @Override + public long parsePointAsMillis(byte[] value) { + return DateUtils.toMilliSeconds(LongPoint.decodeDimension(value, 0)); + } }; private final String type; @@ -141,8 +151,17 @@ NumericType numericType() { */ public abstract Instant toInstant(long value); + /** + * Return the instant that this range can represent that is closest to + * the provided instant. + */ public abstract Instant clampToValidRange(Instant instant); + /** + * Decode the points representation of this field as milliseconds. + */ + public abstract long parsePointAsMillis(byte[] value); + public static Resolution ofOrdinal(int ord) { for (Resolution resolution : values()) { if (ord == resolution.ordinal()) { diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java index d71ed3c3bd97d..93dcdf7be3bda 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java @@ -269,7 +269,8 @@ SortedDocsProducer createSortedDocsProducerOrNull(IndexReader reader, Query quer } return new PointsSortedDocsProducer(fieldType.name(), toBucketFunction, lowerPoint, upperPoint); } else if (fieldType instanceof DateFieldMapper.DateFieldType) { - final ToLongFunction toBucketFunction = (value) -> rounding.applyAsLong(LongPoint.decodeDimension(value, 0)); + ToLongFunction decode = ((DateFieldMapper.DateFieldType) fieldType).resolution()::parsePointAsMillis; + ToLongFunction toBucketFunction = value -> rounding.applyAsLong(decode.applyAsLong(value)); return new PointsSortedDocsProducer(fieldType.name(), toBucketFunction, lowerPoint, upperPoint); } else { return null; diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java index 96563d917bf8f..b8bfae41b2314 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java +++ b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java @@ -18,15 +18,14 @@ */ package org.elasticsearch.search.aggregations.metrics; -import org.apache.lucene.document.LongPoint; import org.apache.lucene.index.IndexOptions; import org.apache.lucene.index.LeafReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.PointValues; import org.apache.lucene.search.CollectionTerminatedException; import org.apache.lucene.search.MatchAllDocsQuery; -import org.apache.lucene.util.Bits; import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.util.Bits; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.DoubleArray; @@ -191,11 +190,7 @@ static Function getPointReaderOrNull(SearchContext context, Aggr converter = ((NumberFieldMapper.NumberFieldType) fieldType)::parsePoint; } else if (fieldType.getClass() == DateFieldMapper.DateFieldType.class) { DateFieldMapper.DateFieldType dft = (DateFieldMapper.DateFieldType) fieldType; - /* - * Makes sure that nanoseconds decode to milliseconds, just - * like they do when you run the agg without the optimization. - */ - converter = (in) -> dft.resolution().toInstant(LongPoint.decodeDimension(in, 0)).toEpochMilli(); + converter = dft.resolution()::parsePointAsMillis; } return converter; } From 28bea413f1384eb5cbe9bc2bc1754edbd2959479 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 10 Mar 2020 10:01:32 -0400 Subject: [PATCH 2/3] Take two --- .../test/search.aggregation/230_composite.yml | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 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 93ad238f8f0b2..bf33877a9eea6 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 @@ -879,6 +879,31 @@ setup: - do: search: index: test + aggregations: + test: + composite: + sources: + - date: + date_histogram: + field: date_nanos + calendar_interval: 1d + format: iso8601 # Format makes the comparisons a little more obvious + aggregations: + avg: + avg: + field: date_nanos + + - match: { hits.total.value: 9 } + - match: { hits.total.relation: eq } + - length: { aggregations.test.buckets: 2 } + - match: { aggregations.test.buckets.0.key.date: "2017-11-21T00:00:00.000Z" } + - match: { aggregations.test.buckets.0.doc_count: 1 } + - match: { aggregations.test.buckets.0.avg.value_as_string: "2017-11-21T01:00:00.000Z" } + - match: { aggregations.test.buckets.1.key.date: "2017-11-22T00:00:00.000Z" } + - match: { aggregations.test.buckets.1.doc_count: 2 } + - match: { aggregations.test.buckets.1.avg.value_as_string: "2017-11-22T01:30:00.000Z" } + +--- "Terms source from sorted": - do: indices.create: @@ -953,27 +978,6 @@ setup: test: composite: sources: -<<<<<<< HEAD - - date: - date_histogram: - field: date_nanos - calendar_interval: 1d - format: iso8601 # Format makes the comparisons a little more obvious - aggregations: - avg: - avg: - field: date_nanos - - - match: { hits.total.value: 9 } - - match: { hits.total.relation: eq } - - length: { aggregations.test.buckets: 2 } - - match: { aggregations.test.buckets.0.key.date: "2017-11-21T00:00:00.000Z" } - - match: { aggregations.test.buckets.0.doc_count: 1 } - - match: { aggregations.test.buckets.0.avg.value_as_string: "2017-11-21T01:00:00.000Z" } - - match: { aggregations.test.buckets.1.key.date: "2017-11-22T00:00:00.000Z" } - - match: { aggregations.test.buckets.1.doc_count: 2 } - - match: { aggregations.test.buckets.1.avg.value_as_string: "2017-11-22T01:30:00.000Z" } -======= - keyword: terms: field: keyword @@ -982,4 +986,3 @@ setup: - length: { aggregations.test.buckets: 1 } - match: { aggregations.test.buckets.0.key.keyword: "foo" } - match: { aggregations.test.buckets.0.doc_count: 1 } ->>>>>>> master From e34c95502f4ccb80139d5fc841f3eb18ceffa1b7 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Tue, 10 Mar 2020 10:32:11 -0400 Subject: [PATCH 3/3] Almost --- .../rest-api-spec/test/search.aggregation/230_composite.yml | 1 + 1 file changed, 1 insertion(+) 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 bf33877a9eea6..1ae5686db3988 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 @@ -879,6 +879,7 @@ setup: - do: search: index: test + body: aggregations: test: composite: