You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2019/02/25 19:26:36 UTC

[GitHub] jihoonson commented on issue #6430: Contributing Moving-Average Query to open source.

jihoonson commented on issue #6430: Contributing Moving-Average Query to open source.
URL: https://github.com/apache/incubator-druid/pull/6430#issuecomment-467147621
 
 
   > I completely agree the work could be incorporated within the general Query. The reason it was created as a separate query type is for the sake of easier implementation (due to the separation of concerns). Calling the underlying query through an internal API is easier than making those changes directly to `Queries` or another class. Perhaps refactoring it into the core query framework could be a future work?
   
   @yurmix I see. It makes sense to me.
   
   > 1. The complexity of `MovingAverageIterable`. But I would be careful in changing that for the sake of refactoring.
   
   It looks complex, but I think it would be easier to understand if you can add more comments. But, simpler implementation would be great if possible. Would you tell me a bit of details of what kind of refactoring you think?
   
   > 2. `Averager` implements their own accumulation formulas (computeResults) which are separate from the formulas of `Aggregator`. I think this could be refactored in order to easily add more types of Averagers and perhaps in order to allign null-handling with the new standards.
   
   It sounds good, but I'm wondering we can use our `Aggregator` for moving average instead of adding a new type of Averager. It would be best if we can because we don't have to implement almost same aggregation logic again (ex, `DoubleMaxAverager` and `DoubleMaxAggregator`). Probably possible if we can integrate `Accumulator` and `Aggregator`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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