Skip to content

Commit 1287df4

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 3369216 commit 1287df4

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

Lines changed: 5 additions & 2 deletions
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
}
@@ -520,7 +523,7 @@ private static long parseUnsignedLong(Object value) {
520523
}
521524

522525
/**
523-
* Convert an unsigned long to the singed long by subtract 2^63 from it
526+
* Convert an unsigned long to the signed long by subtract 2^63 from it
524527
* @param value – unsigned long value in the range [0; 2^64-1], values greater than 2^63-1 are negative
525528
* @return signed long value in the range [-2^63; 2^63-1]
526529
*/

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

Lines changed: 1 addition & 1 deletion
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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram;
2121
import org.elasticsearch.search.aggregations.bucket.range.Range;
2222
import org.elasticsearch.search.aggregations.bucket.terms.Terms;
23+
import org.elasticsearch.search.aggregations.metrics.Min;
2324
import org.elasticsearch.search.aggregations.metrics.Sum;
25+
import org.elasticsearch.search.aggregations.metrics.Max;
2426
import org.elasticsearch.search.sort.SortOrder;
2527
import org.elasticsearch.test.ESIntegTestCase;
2628

@@ -35,6 +37,8 @@
3537
import static org.elasticsearch.search.aggregations.AggregationBuilders.histogram;
3638
import static org.elasticsearch.search.aggregations.AggregationBuilders.range;
3739
import static org.elasticsearch.search.aggregations.AggregationBuilders.sum;
40+
import static org.elasticsearch.search.aggregations.AggregationBuilders.max;
41+
import static org.elasticsearch.search.aggregations.AggregationBuilders.min;
3842
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertSearchResponse;
3943
import static org.hamcrest.Matchers.containsString;
4044
import static org.elasticsearch.search.aggregations.AggregationBuilders.terms;
@@ -279,6 +283,20 @@ public void testAggs() {
279283
double expectedSum = Arrays.stream(values).mapToDouble(Number::doubleValue).sum();
280284
assertEquals(expectedSum, sum.getValue(), 0.001);
281285
}
286+
// max agg
287+
{
288+
SearchResponse response = client().prepareSearch("idx").setSize(0).addAggregation(max("ul_max").field("ul_field")).get();
289+
assertSearchResponse(response);
290+
Max max = response.getAggregations().get("ul_max");
291+
assertEquals(1.8446744073709551615E19, max.getValue(), 0.001);
292+
}
293+
// min agg
294+
{
295+
SearchResponse response = client().prepareSearch("idx").setSize(0).addAggregation(min("ul_min").field("ul_field")).get();
296+
assertSearchResponse(response);
297+
Min min = response.getAggregations().get("ul_min");
298+
assertEquals(0, min.getValue(), 0.001);
299+
}
282300
}
283301

284302
public void testSortDifferentFormatsShouldFail() {

0 commit comments

Comments
 (0)