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 #11124 will be merged, I will try to generify this
function for both `OnheapIncrementalIndex` and `OakIncrementalIndex` (#10001).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
