Skip to content

Commit a63232d

Browse files
authored
Fix date_nanos in composite aggs (#53315)
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
1 parent 146b2a8 commit a63232d

File tree

4 files changed

+76
-8
lines changed

4 files changed

+76
-8
lines changed

rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/230_composite.yml

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ setup:
88
properties:
99
date:
1010
type: date
11+
date_nanos:
12+
type: date_nanos
1113
keyword:
1214
type: keyword
1315
long:
@@ -851,6 +853,57 @@ setup:
851853
- match: { aggregations.test.buckets.0.key.date: "2017-10-21T00:00:00.000-02:00" }
852854
- match: { aggregations.test.buckets.0.doc_count: 2 }
853855

856+
---
857+
"date_histogram on date_nanos":
858+
- skip:
859+
version: " - 7.99.99"
860+
reason: Fixed in 8.0.0 with backport pending to 7.7.0
861+
- do:
862+
index:
863+
index: test
864+
id: 7
865+
body: { "date_nanos": "2017-11-21T01:00:00" }
866+
refresh: true
867+
- do:
868+
index:
869+
index: test
870+
id: 8
871+
body: { "date_nanos": "2017-11-22T01:00:00" }
872+
refresh: true
873+
- do:
874+
index:
875+
index: test
876+
id: 9
877+
body: { "date_nanos": "2017-11-22T02:00:00" }
878+
refresh: true
879+
- do:
880+
search:
881+
index: test
882+
body:
883+
aggregations:
884+
test:
885+
composite:
886+
sources:
887+
- date:
888+
date_histogram:
889+
field: date_nanos
890+
calendar_interval: 1d
891+
format: iso8601 # Format makes the comparisons a little more obvious
892+
aggregations:
893+
avg:
894+
avg:
895+
field: date_nanos
896+
897+
- match: { hits.total.value: 9 }
898+
- match: { hits.total.relation: eq }
899+
- length: { aggregations.test.buckets: 2 }
900+
- match: { aggregations.test.buckets.0.key.date: "2017-11-21T00:00:00.000Z" }
901+
- match: { aggregations.test.buckets.0.doc_count: 1 }
902+
- match: { aggregations.test.buckets.0.avg.value_as_string: "2017-11-21T01:00:00.000Z" }
903+
- match: { aggregations.test.buckets.1.key.date: "2017-11-22T00:00:00.000Z" }
904+
- match: { aggregations.test.buckets.1.doc_count: 2 }
905+
- match: { aggregations.test.buckets.1.avg.value_as_string: "2017-11-22T01:30:00.000Z" }
906+
854907
---
855908
"Terms source from sorted":
856909
- do:

server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,11 @@ public Instant toInstant(long value) {
9797
public Instant clampToValidRange(Instant instant) {
9898
return instant;
9999
}
100+
101+
@Override
102+
public long parsePointAsMillis(byte[] value) {
103+
return LongPoint.decodeDimension(value, 0);
104+
}
100105
},
101106
NANOSECONDS(DATE_NANOS_CONTENT_TYPE, NumericType.DATE_NANOSECONDS) {
102107
@Override
@@ -113,6 +118,11 @@ public Instant toInstant(long value) {
113118
public Instant clampToValidRange(Instant instant) {
114119
return DateUtils.clampToNanosRange(instant);
115120
}
121+
122+
@Override
123+
public long parsePointAsMillis(byte[] value) {
124+
return DateUtils.toMilliSeconds(LongPoint.decodeDimension(value, 0));
125+
}
116126
};
117127

118128
private final String type;
@@ -141,8 +151,17 @@ NumericType numericType() {
141151
*/
142152
public abstract Instant toInstant(long value);
143153

154+
/**
155+
* Return the instant that this range can represent that is closest to
156+
* the provided instant.
157+
*/
144158
public abstract Instant clampToValidRange(Instant instant);
145159

160+
/**
161+
* Decode the points representation of this field as milliseconds.
162+
*/
163+
public abstract long parsePointAsMillis(byte[] value);
164+
146165
public static Resolution ofOrdinal(int ord) {
147166
for (Resolution resolution : values()) {
148167
if (ord == resolution.ordinal()) {

server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/LongValuesSource.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ SortedDocsProducer createSortedDocsProducerOrNull(IndexReader reader, Query quer
269269
}
270270
return new PointsSortedDocsProducer(fieldType.name(), toBucketFunction, lowerPoint, upperPoint);
271271
} else if (fieldType instanceof DateFieldMapper.DateFieldType) {
272-
final ToLongFunction<byte[]> toBucketFunction = (value) -> rounding.applyAsLong(LongPoint.decodeDimension(value, 0));
272+
ToLongFunction<byte[]> decode = ((DateFieldMapper.DateFieldType) fieldType).resolution()::parsePointAsMillis;
273+
ToLongFunction<byte[]> toBucketFunction = value -> rounding.applyAsLong(decode.applyAsLong(value));
273274
return new PointsSortedDocsProducer(fieldType.name(), toBucketFunction, lowerPoint, upperPoint);
274275
} else {
275276
return null;

server/src/main/java/org/elasticsearch/search/aggregations/metrics/MinAggregator.java

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,14 @@
1818
*/
1919
package org.elasticsearch.search.aggregations.metrics;
2020

21-
import org.apache.lucene.document.LongPoint;
2221
import org.apache.lucene.index.IndexOptions;
2322
import org.apache.lucene.index.LeafReader;
2423
import org.apache.lucene.index.LeafReaderContext;
2524
import org.apache.lucene.index.PointValues;
2625
import org.apache.lucene.search.CollectionTerminatedException;
2726
import org.apache.lucene.search.MatchAllDocsQuery;
28-
import org.apache.lucene.util.Bits;
2927
import org.apache.lucene.search.ScoreMode;
28+
import org.apache.lucene.util.Bits;
3029
import org.elasticsearch.common.lease.Releasables;
3130
import org.elasticsearch.common.util.BigArrays;
3231
import org.elasticsearch.common.util.DoubleArray;
@@ -191,11 +190,7 @@ static Function<byte[], Number> getPointReaderOrNull(SearchContext context, Aggr
191190
converter = ((NumberFieldMapper.NumberFieldType) fieldType)::parsePoint;
192191
} else if (fieldType.getClass() == DateFieldMapper.DateFieldType.class) {
193192
DateFieldMapper.DateFieldType dft = (DateFieldMapper.DateFieldType) fieldType;
194-
/*
195-
* Makes sure that nanoseconds decode to milliseconds, just
196-
* like they do when you run the agg without the optimization.
197-
*/
198-
converter = (in) -> dft.resolution().toInstant(LongPoint.decodeDimension(in, 0)).toEpochMilli();
193+
converter = dft.resolution()::parsePointAsMillis;
199194
}
200195
return converter;
201196
}

0 commit comments

Comments
 (0)