leventov opened a new issue #7287: Refactor AggregatorFactory interface into two different interfaces, return the second one from getCombiningFactory() URL: https://github.com/apache/incubator-druid/issues/7287 Currently, `AggregatorFactory.getCombiningFactory()` and `getMergingFactory()` should return an instance of `AggregatorFactory` interface, which has a lot of methods that are common with the base `AggregatorFactory`. It provokes developers to "cut the corner" by extending the base `AggregatorFactory`. But they often forget to override `makeAggregateCombiner()` method, see #6039 and #7243. Solution: extract a separate aggregating interface that has only aggregation-related methods and methods that manipulate the objects, but doesn't have `getName()`, `requiredFields()` (?), `getTypeName()` (?), and `getMergingFactory()` and `getCombiningFactory()` themselves. Then this secondary interface is returned from methods `getCombiningFactory()` and `getAggregationFactory()` (a new method). It may be that this secondary interface should better be called "AggregatorFactory" (like the former conflated interface), while the basic interface should be renamed somehow. Other issues that might be related: #7019 and #6858.
---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org For additional commands, e-mail: commits-h...@druid.apache.org
