gianm commented on a change in pull request #7496: refactor druid-bloom-filter aggregators URL: https://github.com/apache/incubator-druid/pull/7496#discussion_r276719274
########## File path: extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/aggregation/bloom/DoubleBloomFilterAggregator.java ########## @@ -23,20 +23,22 @@ import org.apache.druid.query.filter.BloomKFilter; import org.apache.druid.segment.BaseDoubleColumnValueSelector; +import java.nio.ByteBuffer; + public final class DoubleBloomFilterAggregator extends BaseBloomFilterAggregator<BaseDoubleColumnValueSelector> Review comment: By the way, one original reason for the CardinalityAggregator-style design choice is that the logic for choosing which implementation to use could be centralized / standardized. The equivalent of this code here: https://github.com/apache/incubator-druid/pull/7496/files#diff-06cafba60d560f4a5bb1551a56b8041dR99 is this code in CardinalityAggregatorFactory: ```java ColumnSelectorPlus<CardinalityAggregatorColumnSelectorStrategy>[] selectorPluses = DimensionHandlerUtils.createColumnSelectorPluses( STRATEGY_FACTORY, fields, columnFactory ); ``` Which calls out to a standard library function. One other cool thing we can do here is modify the ColumnSelectorStrategyFactory interface to look more like this: ```java public interface ColumnStrategizer<T> { T makeDimensionStrategy(DimensionSelector selector); T makeFloatStrategy(ColumnValueSelector selector); T makeDoubleStrategy(ColumnValueSelector selector); T makeLongStrategy(ColumnValueSelector selector); } ``` That way, there's a structure in place to make sure every type has some sort of handling. Or if we used default methods, at least consistent exceptions being thrown when types are missing handling. It isn't like this yet, but it could be, and I think it would be nice. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
