[GitHub] [druid] liran-funaro commented on a change in pull request #11160: Remove IncrementalIndex template modifier

[GitBox <gi...@apache.org>]

liran-funaro commented on a change in pull request #11160:
URL: https://github.com/apache/druid/pull/11160#discussion_r736285060



##########
File path: processing/src/main/java/org/apache/druid/segment/incremental/OnheapIncrementalIndex.java
##########
@@ -369,6 +367,61 @@ public boolean isNull(int rowOffset, int aggOffset)
     return concurrentGet(rowOffset)[aggOffset].isNull();
   }
 
+  @Override
+  public Iterable<Row> iterableWithPostAggregations(

Review comment:
       When this method was in the generic `IncrementalIndex`, it has to use the generic `AggregatorType` by calling the following abstract generic functions:
   ```java
   AggregatorType[] getAggsForRow(int rowOffset);
   Object getAggVal(AggregatorType agg, int rowOffset, int aggPosition);
   ```
   These functions were forced and didn't really fit each of the implementations.
   `getAggsForRow()` didn't fit `OffheapIncrementalIndex` since it only have one aggregator instance (not one for each row).
   `getAggVal()` didn't fit `OnheapIncrementalIndex` since `rowOffset` and `aggPosition` is only relevant to `BufferAggregator`.
   
   That said, other than that, the implementation is identical in both incremental-index implementations.
   It might be possible to find a more generic implementation. But since we are removing `OffheapIncrementalIndex`, I don't see the point of making this effort here.
   Once this PR and PR#1124 will be merged, I will try to generify this function for both `OnheapIncrementalIndex` and `OakIncrementalIndex`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org