Skip to content

Commit 9a77434

Browse files
committed
Serialize sum as null instead of 0.0 for empty histograms.
1 parent 28f611e commit 9a77434

File tree

5 files changed

+45
-17
lines changed

5 files changed

+45
-17
lines changed

libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContent.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ public static void serialize(XContentBuilder builder, @Nullable ExponentialHisto
6363
builder.startObject();
6464

6565
builder.field(SCALE_FIELD, histogram.scale());
66-
builder.field(SUM_FIELD, histogram.sum());
66+
67+
if (histogram.sum() != 0.0 || histogram.valueCount() > 0) {
68+
builder.field(SUM_FIELD, histogram.sum());
69+
}
6770
if (Double.isNaN(histogram.min()) == false) {
6871
builder.field(MIN_FIELD, histogram.min());
6972
}

libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public void testNullHistogram() {
4040

4141
public void testEmptyHistogram() {
4242
ExponentialHistogram emptyHistogram = ExponentialHistogram.empty();
43-
assertThat(toJson(emptyHistogram), equalTo("{\"scale\":" + emptyHistogram.scale() + ",\"sum\":0.0}"));
43+
assertThat(toJson(emptyHistogram), equalTo("{\"scale\":" + emptyHistogram.scale() + "}"));
4444
}
4545

4646
public void testFullHistogram() {

x-pack/plugin/esql/qa/testFixtures/src/main/resources/exponential_histogram.csv-spec

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ FROM exp_histo_sample | WHERE STARTS_WITH(instance, "dummy") | SORT instance | K
55
;
66

77
instance:keyword | responseTime:exponential_histogram
8-
dummy-empty | "{""scale"":-7,""sum"":0.0}"
8+
dummy-empty | "{""scale"":-7}"
99
dummy-full | "{""scale"":0,""sum"":-3775.0,""min"":-100.0,""max"":50.0,""zero"":{""count"":1,""threshold"":1.0E-4},""positive"":{""indices"":[-1,0,1,2,3,4,5],""counts"":[1,1,2,4,8,16,18]},""negative"":{""indices"":[-1,0,1,2,3,4,5,6],""counts"":[1,1,2,4,8,16,32,36]}}"
1010
dummy-negative_only | "{""scale"":2,""sum"":-1275.0,""min"":-50.0,""max"":-1.0,""negative"":{""indices"":[-1,3,6,7,9,10,11,12,13,14,15,16,17,18,19,20,21,22],""counts"":[1,1,1,1,1,1,2,1,2,2,3,3,3,4,6,6,7,5]}}"
1111
dummy-no_zero_bucket | "{""scale"":0,""sum"":-3775.0,""min"":-100.0,""max"":50.0,""positive"":{""indices"":[-1,0,1,2,3,4,5],""counts"":[1,1,2,4,8,16,18]},""negative"":{""indices"":[-1,0,1,2,3,4,5,6],""counts"":[1,1,2,4,8,16,32,36]}}"
1212
dummy-positive_only | "{""scale"":2,""sum"":1275.0,""min"":1.0,""max"":50.0,""positive"":{""indices"":[-1,3,6,7,9,10,11,12,13,14,15,16,17,18,19,20,21,22],""counts"":[1,1,1,1,1,1,2,1,2,2,3,3,3,4,6,6,7,5]}}"
1313
dummy-zero_count_only | "{""scale"":2,""sum"":0.0,""min"":0.0,""max"":0.0,""zero"":{""count"":101}}"
14-
dummy-zero_threshold_only | "{""scale"":0,""sum"":0.0,""zero"":{""threshold"":2.0E-5}}"
14+
dummy-zero_threshold_only | "{""scale"":0,""zero"":{""threshold"":2.0E-5}}"
1515
;
1616

1717

x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -568,21 +568,30 @@ private Map<String, Object> convertHistogramToCanonicalForm(Map<String, Object>
568568

569569
Map<String, Object> zeroBucket = convertZeroBucketToCanonicalForm(Types.forciblyCast(histogram.get("zero")));
570570

571-
Object sum = histogram.get("sum");
572-
if (sum == null) {
573-
sum = ExponentialHistogramUtils.estimateSum(
574-
IndexWithCount.asBuckets(scale, negative).iterator(),
575-
IndexWithCount.asBuckets(scale, positive).iterator()
576-
);
571+
Number sum = (Number) histogram.get("sum");
572+
ExponentialHistogram.Buckets negativeBuckets = IndexWithCount.asBuckets(scale, negative);
573+
ExponentialHistogram.Buckets positiveBuckets = IndexWithCount.asBuckets(scale, positive);
574+
575+
boolean isEmpty = negativeBuckets.iterator().hasNext() == false
576+
&& positiveBuckets.iterator().hasNext() == false
577+
&& (zeroBucket == null || Types.<Number>forciblyCast(zeroBucket.getOrDefault("count", 0L)).longValue() == 0L);
578+
579+
// we allow 0.0 as sum for input histograms, but output null in canonical form in that case
580+
if (isEmpty && (sum == null || sum.doubleValue() == 0.0)) {
581+
sum = null;
582+
} else if (sum == null) {
583+
sum = ExponentialHistogramUtils.estimateSum(negativeBuckets.iterator(), positiveBuckets.iterator());
584+
}
585+
if (sum != null) {
586+
result.put("sum", sum);
577587
}
578-
result.put("sum", sum);
579588

580589
Object min = histogram.get("min");
581590
if (min == null) {
582591
OptionalDouble estimatedMin = ExponentialHistogramUtils.estimateMin(
583592
mapToZeroBucket(zeroBucket),
584-
IndexWithCount.asBuckets(scale, negative),
585-
IndexWithCount.asBuckets(scale, positive)
593+
negativeBuckets,
594+
positiveBuckets
586595
);
587596
if (estimatedMin.isPresent()) {
588597
min = estimatedMin.getAsDouble();
@@ -596,8 +605,8 @@ private Map<String, Object> convertHistogramToCanonicalForm(Map<String, Object>
596605
if (max == null) {
597606
OptionalDouble estimatedMax = ExponentialHistogramUtils.estimateMax(
598607
mapToZeroBucket(zeroBucket),
599-
IndexWithCount.asBuckets(scale, negative),
600-
IndexWithCount.asBuckets(scale, positive)
608+
negativeBuckets,
609+
positiveBuckets
601610
);
602611
if (estimatedMax.isPresent()) {
603612
max = estimatedMax.getAsDouble();

x-pack/plugin/mapper-exponential-histogram/src/yamlRestTest/resources/rest-api-spec/test/10_synthetic_source.yml

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,24 @@ setup:
161161
counts: [3, 2, 1]
162162
---
163163
"Empty histogram":
164+
- do:
165+
index:
166+
index: test_exponential_histogram
167+
id: "1"
168+
refresh: true
169+
body:
170+
my_histo:
171+
scale: -7
172+
- do:
173+
get:
174+
index: test_exponential_histogram
175+
id: "1"
176+
- match:
177+
_source.my_histo:
178+
scale: -7
179+
180+
---
181+
"Empty histogram with sum":
164182
- do:
165183
index:
166184
index: test_exponential_histogram
@@ -176,7 +194,6 @@ setup:
176194
id: "1"
177195
- match:
178196
_source.my_histo:
179-
sum: 0.0
180197
scale: -7
181198

182199
---
@@ -198,7 +215,6 @@ setup:
198215
- match:
199216
_source.my_histo:
200217
scale: 0
201-
sum: 0.0
202218
zero:
203219
threshold: 42.7
204220

0 commit comments

Comments
 (0)