Skip to content

Commit 072da36

Browse files
Fix max/min aggs for unsigned_long (#63904)
Max and min aggs were producing wrong results for unsigned_long field if field was indexed. If field is indexed for max/min aggs instead of field data, we use values from indexed Points, values of which are derived using method pointReaderIfPossible. Before UnsignedLongFieldType#pointReaderIfPossible was incorrectly producing values, as it failed to shift them back to original values. This patch fixes method pointReaderIfPossible to produce correct original values. Relates to #60050
1 parent 3deebc2 commit 072da36

File tree

3 files changed

+24
-3
lines changed

3 files changed

+24
-3
lines changed

x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongFieldMapper.java

+5-2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
import java.util.function.Function;
5050
import java.util.function.Supplier;
5151

52+
import static org.elasticsearch.xpack.unsignedlong.UnsignedLongLeafFieldData.convertUnsignedLongToDouble;
53+
5254
public class UnsignedLongFieldMapper extends ParametrizedFieldMapper {
5355
public static final String CONTENT_TYPE = "unsigned_long";
5456

@@ -272,7 +274,8 @@ public DocValueFormat docValueFormat(String format, ZoneId timeZone) {
272274
@Override
273275
public Function<byte[], Number> pointReaderIfPossible() {
274276
if (isSearchable()) {
275-
return (value) -> LongPoint.decodeDimension(value, 0);
277+
// convert from the shifted value back to the original value
278+
return (value) -> convertUnsignedLongToDouble(LongPoint.decodeDimension(value, 0));
276279
}
277280
return null;
278281
}
@@ -525,7 +528,7 @@ private static long parseUnsignedLong(Object value) {
525528
}
526529

527530
/**
528-
* Convert an unsigned long to the singed long by subtract 2^63 from it
531+
* Convert an unsigned long to the signed long by subtract 2^63 from it
529532
* @param value – unsigned long value in the range [0; 2^64-1], values greater than 2^63-1 are negative
530533
* @return signed long value in the range [-2^63; 2^63-1]
531534
*/

x-pack/plugin/mapper-unsigned-long/src/main/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongLeafFieldData.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ public Object nextValue() throws IOException {
112112
};
113113
}
114114

115-
private static double convertUnsignedLongToDouble(long value) {
115+
static double convertUnsignedLongToDouble(long value) {
116116
if (value < 0L) {
117117
return sortableSignedLongToUnsigned(value); // add 2 ^ 63
118118
} else {

x-pack/plugin/mapper-unsigned-long/src/test/java/org/elasticsearch/xpack/unsignedlong/UnsignedLongTests.java

+18
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
2222
import org.elasticsearch.search.aggregations.bucket.range.Range;
2323
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
24+
import org.elasticsearch.search.aggregations.metrics.Min;
2425
import org.elasticsearch.search.aggregations.metrics.Sum;
26+
import org.elasticsearch.search.aggregations.metrics.Max;
2527
import org.elasticsearch.search.sort.SortOrder;
2628
import org.elasticsearch.test.ESIntegTestCase;
2729

@@ -36,6 +38,8 @@
3638
import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram;
3739
import static org.elasticsearch.search.aggregations.AggregationBuilders.range;
3840
import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
41+
import static org.elasticsearch.search.aggregations.AggregationBuilders.max;
42+
import static org.elasticsearch.search.aggregations.AggregationBuilders.min;
3943
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
4044
import static org.hamcrest.Matchers.containsString;
4145
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
@@ -273,6 +277,20 @@ public void testAggs() {
273277
double expectedSum = Arrays.stream(values).mapToDouble(Number::doubleValue).sum();
274278
assertEquals(expectedSum, sum.getValue(), 0.001);
275279
}
280+
// max agg
281+
{
282+
SearchResponse response = client().prepareSearch("idx").setSize(0).addAggregation(max("ul_max").field("ul_field")).get();
283+
assertSearchResponse(response);
284+
Max max = response.getAggregations().get("ul_max");
285+
assertEquals(1.8446744073709551615E19, max.getValue(), 0.001);
286+
}
287+
// min agg
288+
{
289+
SearchResponse response = client().prepareSearch("idx").setSize(0).addAggregation(min("ul_min").field("ul_field")).get();
290+
assertSearchResponse(response);
291+
Min min = response.getAggregations().get("ul_min");
292+
assertEquals(0, min.getValue(), 0.001);
293+
}
276294
}
277295

278296
public void testSortDifferentFormatsShouldFail() {

0 commit comments

Comments
 (0)