Skip to content

Commit aebedd6

Browse files
authored
Fix incorrect exponential histogram sub-block serialization. (#137279)
1 parent 8436d0c commit aebedd6

File tree

3 files changed

+60
-15
lines changed

3 files changed

+60
-15
lines changed

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ExponentialHistogramArrayBlock.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,12 @@ public ExponentialHistogramArrayBlock deepCopy(BlockFactory blockFactory) {
291291

292292
@Override
293293
public void writeTo(StreamOutput out) throws IOException {
294-
minima.writeTo(out);
295-
maxima.writeTo(out);
296-
sums.writeTo(out);
297-
valueCounts.writeTo(out);
298-
zeroThresholds.writeTo(out);
299-
encodedHistograms.writeTo(out);
294+
Block.writeTypedBlock(minima, out);
295+
Block.writeTypedBlock(maxima, out);
296+
Block.writeTypedBlock(sums, out);
297+
Block.writeTypedBlock(valueCounts, out);
298+
Block.writeTypedBlock(zeroThresholds, out);
299+
Block.writeTypedBlock(encodedHistograms, out);
300300
}
301301

302302
public static ExponentialHistogramArrayBlock readFrom(BlockStreamInput in) throws IOException {
@@ -309,12 +309,12 @@ public static ExponentialHistogramArrayBlock readFrom(BlockStreamInput in) throw
309309

310310
boolean success = false;
311311
try {
312-
minima = DoubleBlock.readFrom(in);
313-
maxima = DoubleBlock.readFrom(in);
314-
sums = DoubleBlock.readFrom(in);
315-
valueCounts = LongBlock.readFrom(in);
316-
zeroThresholds = DoubleBlock.readFrom(in);
317-
encodedHistograms = BytesRefBlock.readFrom(in);
312+
minima = (DoubleBlock) Block.readTypedBlock(in);
313+
maxima = (DoubleBlock) Block.readTypedBlock(in);
314+
sums = (DoubleBlock) Block.readTypedBlock(in);
315+
valueCounts = (LongBlock) Block.readTypedBlock(in);
316+
zeroThresholds = (DoubleBlock) Block.readTypedBlock(in);
317+
encodedHistograms = (BytesRefBlock) Block.readTypedBlock(in);
318318
success = true;
319319
} finally {
320320
if (success == false) {
Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,65 @@
77

88
package org.elasticsearch.compute.data;
99

10+
import org.elasticsearch.common.io.stream.BytesStreamOutput;
11+
import org.elasticsearch.compute.test.BlockTestUtils;
1012
import org.elasticsearch.compute.test.ComputeTestCase;
1113
import org.elasticsearch.compute.test.RandomBlock;
1214
import org.elasticsearch.core.Releasables;
1315
import org.elasticsearch.exponentialhistogram.ExponentialHistogram;
1416
import org.elasticsearch.exponentialhistogram.ExponentialHistogramCircuitBreaker;
1517

18+
import java.io.IOException;
1619
import java.util.List;
1720

1821
import static org.hamcrest.CoreMatchers.not;
1922
import static org.hamcrest.Matchers.equalTo;
2023

21-
public class ExponentialHistogramBlockEqualityTests extends ComputeTestCase {
24+
public class ExponentialHistogramBlockTests extends ComputeTestCase {
2225

23-
public void testEmptyBlock() {
26+
public void testPopulatedBlockSerialization() throws IOException {
27+
int elementCount = randomIntBetween(1, 100);
28+
ExponentialHistogramBlockBuilder builder = blockFactory().newExponentialHistogramBlockBuilder(elementCount);
29+
for (int i = 0; i < elementCount; i++) {
30+
if (randomBoolean()) {
31+
builder.appendNull();
32+
} else {
33+
builder.append(BlockTestUtils.randomExponentialHistogram());
34+
}
35+
}
36+
ExponentialHistogramBlock block = builder.build();
37+
Block deserializedBlock = serializationRoundTrip(block);
38+
assertThat(deserializedBlock, equalTo(block));
39+
Releasables.close(block, deserializedBlock);
40+
}
41+
42+
public void testNullSerialization() throws IOException {
43+
// sub-blocks can be constant null, those should serialize correctly too
44+
int elementCount = randomIntBetween(1, 100);
45+
46+
Block block = new ExponentialHistogramArrayBlock(
47+
(DoubleBlock) blockFactory().newConstantNullBlock(elementCount),
48+
(DoubleBlock) blockFactory().newConstantNullBlock(elementCount),
49+
(DoubleBlock) blockFactory().newConstantNullBlock(elementCount),
50+
(LongBlock) blockFactory().newConstantNullBlock(elementCount),
51+
(DoubleBlock) blockFactory().newConstantNullBlock(elementCount),
52+
(BytesRefBlock) blockFactory().newConstantNullBlock(elementCount)
53+
);
54+
55+
Block deserializedBlock = serializationRoundTrip(block);
56+
assertThat(deserializedBlock, equalTo(block));
57+
Releasables.close(block, deserializedBlock);
58+
}
59+
60+
private Block serializationRoundTrip(Block block) throws IOException {
61+
BytesStreamOutput out = new BytesStreamOutput();
62+
Block.writeTypedBlock(block, out);
63+
try (BlockStreamInput input = new BlockStreamInput(out.bytes().streamInput(), blockFactory())) {
64+
return Block.readTypedBlock(input);
65+
}
66+
}
67+
68+
public void testEmptyBlockEquality() {
2469
List<Block> blocks = List.of(
2570
blockFactory().newConstantNullBlock(0),
2671
blockFactory().newExponentialHistogramBlockBuilder(0).build(),

x-pack/plugin/esql/compute/test/src/main/java/org/elasticsearch/compute/test/BlockTestUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ public static Page convertBytesRefsToOrdinals(Page page) {
374374
}
375375
}
376376

377-
static ExponentialHistogram randomExponentialHistogram() {
377+
public static ExponentialHistogram randomExponentialHistogram() {
378378
// TODO(b/133393): allow (index,scale) based zero thresholds as soon as we support them in the block
379379
// ideally Replace this with the shared random generation in ExponentialHistogramTestUtils
380380
boolean hasNegativeValues = randomBoolean();

0 commit comments

Comments
 (0)