Add TODOs

Author: jan-elastic

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/ChangePointLongAggregator.java

@@ -37,15 +37,20 @@
 import java.util.function.Function;
 
 /**
- * Aggregates field values for long.
- * TODO: make .java.st from this to support other types
- * TODO: add "includeTimestamp" to @Aggregator
+ * Change point detection for series of long values.
  */
-// TODO: add normal @Aggregator
-// @Aggregator({
-// includeTimestamps = true,
-// @IntermediateState(name = "timestamps", type = "LONG_BLOCK"),
-// @IntermediateState(name = "values", type = "LONG_BLOCK") })
+
+// TODO: make .java.st from this to support different types
+
+// TODO: add non-grouping @Aggregator, like this:
+/*
+@Aggregator(
+    includeTimestamps = true,
+    value = { @IntermediateState(name = "timestamps", type = "LONG_BLOCK"), @IntermediateState(name = "values", type = "LONG_BLOCK") }
+)
+*/
+// This need "includeTimestamps" support in @Aggregator.
+
 @GroupingAggregator(
     includeTimestamps = true,
     value = { @IntermediateState(name = "timestamps", type = "LONG_BLOCK"), @IntermediateState(name = "values", type = "LONG_BLOCK") }
@@ -123,7 +128,7 @@ public int compareTo(TimeAndValue other) {
         }
 
         void sort() {
-            // TODO: this is very inefficient and doesn't account for memory!
+            // TODO: this copying is a bit inefficient and doesn't account for memory
             List<TimeAndValue> list = new ArrayList<>(count);
             for (int i = 0; i < count; i++) {
                 list.add(new TimeAndValue(timestamps.get(i), values.get(i)));
@@ -185,6 +190,7 @@ void toIntermediate(Block[] blocks, int offset, IntVector selected, DriverContex
         }
 
         public Block evaluateFinal(IntVector selected, BlockFactory blockFactory) {
+            // TODO: this needs to output multiple columns or a composite object, not a JSON blob.
             try (BytesRefBlock.Builder builder = blockFactory.newBytesRefBlockBuilder(selected.getPositionCount())) {
                 for (int s = 0; s < selected.getPositionCount(); s++) {
                     SingleState state = states.get(selected.getInt(s));
@@ -244,7 +250,6 @@ Block toBlock(Function<SingleState, LongArray> getArray, BlockFactory blockFacto
         }
 
         void enableGroupIdTracking(SeenGroupIds seenGroupIds) {
-            // noop - we handle the null states inside `toIntermediate` and `evaluateFinal`
         }
 
         @Override

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/ChangePoint.java

@@ -36,6 +36,7 @@ public class ChangePoint extends AggregateFunction implements ToAggregator {
         ChangePoint::new
     );
 
+    // TODO: make "timestamp" field optional
     @FunctionInfo(returnType = { "string" }, description = "...", isAggregation = true)
     public ChangePoint(
         Source source,
@@ -77,6 +78,7 @@ public ChangePoint replaceChildren(List<Expression> newChildren) {
         return new ChangePoint(source(), newChildren.get(0), newChildren.get(1), newChildren.get(2));
     }
 
+    // TODO: this needs to output multiple columns or a composite object
     @Override
     public DataType dataType() {
         return DataType.KEYWORD;
@@ -103,6 +105,7 @@ public AggregatorFunctionSupplier supplier(List<Integer> inputChannels) {
         final DataType type = field().dataType();
         return switch (type) {
             case LONG -> new ChangePointLongAggregatorFunctionSupplier(inputChannels);
+            // TODO: support other types
             default -> throw EsqlIllegalArgumentException.illegalDataType(type);
         };
     }