Review fixes

Author: JonasKunz

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

@@ -19,9 +19,34 @@
 
 final class ExponentialHistogramArrayBlock extends AbstractNonThreadSafeRefCounted implements ExponentialHistogramBlock {
 
+    // Exponential histograms consist of several components that we store in separate blocks
+    // due to (a) better compression in the field mapper for disk storage and (b) faster computations if only one sub-component is needed
+    // What are the semantics of positions, multi-value counts and nulls in the exponential histogram block and
+    // how do they relate to the sub-blocks?
+    // ExponentialHistogramBlock need to adhere to the contract of Blocks for the access patterns:
+    //
+    // for (int position = 0; position < block.getPositionCount(); position++) {
+    // ...int valueCount = block.getValueCount(position);
+    // ...for (int valueIndex = 0; valueIndex < valueCount; valueIndex++) {
+    // ......ExponentialHistogram histo = block.getExponentialHistogram(valueIndex, scratch);
+    // ...}
+    // }
+    //
+    // That implies that given only a value-index, we need to be able to retrieve all components of the histogram.
+    // Because we can't make any assumptions on how value indices are laid out in the sub-blocks for multi-values,
+    // we enforce that the sub-blocks have at most one value per position (i.e., no multi-values).
+    // Based on this, we can define the valueIndex for ExponentialHistogramArrayBlock to correspond to positions in the sub-blocks.
+    // So basically the sub-blocks are the "flattened" components of the histograms.
+    // If we later add multi-value support to ExponentialHistogramArrayBlock,
+    // we can't use the multi-value support of the sub-blocks to implement that.
+    // Instead, we need to maintain a firstValueIndex array ourselves in ExponentialHistogramArrayBlock.
+
     private final DoubleBlock minima;
     private final DoubleBlock maxima;
     private final DoubleBlock sums;
+    /**
+     Holds the number of values in each histogram. Note that this is a different concept from getValueCount(position)!
+     */
     private final LongBlock valueCounts;
     private final DoubleBlock zeroThresholds;
     private final BytesRefBlock encodedHistograms;
@@ -96,6 +121,8 @@ public ExponentialHistogram getExponentialHistogram(int valueIndex, ExponentialH
 
     @Override
     public void serializeExponentialHistogram(int valueIndex, SerializedOutput out, BytesRef scratch) {
+        // not that this value count is different from getValueCount(position)!
+        // this value count represents the number of individual samples the histogram was computed for
         long valueCount = valueCounts.getLong(valueCounts.getFirstValueIndex(valueIndex));
         out.appendLong(valueCounts.getLong(valueCounts.getFirstValueIndex(valueIndex)));
         out.appendDouble(sums.getDouble(sums.getFirstValueIndex(valueIndex)));

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

@@ -73,7 +73,7 @@ interface SerializedInput {
 
         long readLong();
 
-        BytesRef readBytesRef(BytesRef tempBytesRef);
+        BytesRef readBytesRef(BytesRef scratch);
     }
 
 }

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/ResultBuilderForExponentialHistogram.java

@@ -54,7 +54,7 @@ public void close() {
         builder.close();
     }
 
-    private class ReusableTopNEncoderInput implements ExponentialHistogramBlock.SerializedInput {
+    private static final class ReusableTopNEncoderInput implements ExponentialHistogramBlock.SerializedInput {
         BytesRef inputValues;
 
         @Override
@@ -68,8 +68,8 @@ public long readLong() {
         }
 
         @Override
-        public BytesRef readBytesRef(BytesRef tempBytesRef) {
-            return TopNEncoder.DEFAULT_UNSORTABLE.decodeBytesRef(inputValues, tempBytesRef);
+        public BytesRef readBytesRef(BytesRef scratch) {
+            return TopNEncoder.DEFAULT_UNSORTABLE.decodeBytesRef(inputValues, scratch);
         }
     }
 }