[GitHub] jihoonson commented on a change in pull request #5857: Optimize filtered aggs with interval filters in per-segment queries
jihoonson commented on a change in pull request #5857: Optimize filtered aggs
with interval filters in per-segment queries
URL: https://github.com/apache/incubator-druid/pull/5857#discussion_r206664109
##
File path:
processing/src/main/java/io/druid/query/aggregation/FilteredAggregatorFactory.java
##
@@ -140,6 +144,61 @@ public int getMaxIntermediateSize()
return delegate.getMaxIntermediateSize();
}
+ @Override
+ public AggregatorFactory
optimizeForSegment(PerSegmentQueryOptimizationContext optimizationContext)
+ {
+if (filter instanceof IntervalDimFilter) {
Review comment:
Sounds nice!
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:
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
[GitHub] jihoonson commented on a change in pull request #5857: Optimize filtered aggs with interval filters in per-segment queries
jihoonson commented on a change in pull request #5857: Optimize filtered aggs
with interval filters in per-segment queries
URL: https://github.com/apache/incubator-druid/pull/5857#discussion_r206304373
##
File path:
processing/src/main/java/io/druid/query/aggregation/FilteredAggregatorFactory.java
##
@@ -140,6 +144,61 @@ public int getMaxIntermediateSize()
return delegate.getMaxIntermediateSize();
}
+ @Override
+ public AggregatorFactory
optimizeForSegment(PerSegmentQueryOptimizationContext optimizationContext)
+ {
+if (filter instanceof IntervalDimFilter) {
Review comment:
It looks that this optimization would work only when there is a single
`IntervalDimFilter` here. But, probably there would be more complicated filter
using `AndDimFilter` or `OrDimFilter`. To support that, we need to check all
filters. What do you think about adding a new interface like
`optimizeForSegment` to `DimFilter`? Probably we also need to use the visitor
pattern to fully optimize this.
I'm not sure this should be a part of this PR. If you think it's not, would
you please raise an issue about this and add a link here?
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:
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
[GitHub] jihoonson commented on a change in pull request #5857: Optimize filtered aggs with interval filters in per-segment queries
jihoonson commented on a change in pull request #5857: Optimize filtered aggs with interval filters in per-segment queries URL: https://github.com/apache/incubator-druid/pull/5857#discussion_r206301783 ## File path: processing/src/main/java/io/druid/query/aggregation/AggregatorFactory.java ## @@ -141,6 +142,17 @@ public AggregatorFactory getMergingFactory(AggregatorFactory other) throws Aggre */ public abstract int getMaxIntermediateSize(); + /** + * Return a potentially optimized form of this AggregatorFactory for per-segment queries. + * + * @param optimizationContext + * @return Review comment: nit: you can remove `@param` and `@return` here. 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: 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
[GitHub] jihoonson commented on a change in pull request #5857: Optimize filtered aggs with interval filters in per-segment queries
jihoonson commented on a change in pull request #5857: Optimize filtered aggs with interval filters in per-segment queries URL: https://github.com/apache/incubator-druid/pull/5857#discussion_r206301554 ## File path: processing/src/main/java/io/druid/query/PerSegmentQueryOptimizationContext.java ## @@ -0,0 +1,37 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.query; + +public class PerSegmentQueryOptimizationContext Review comment: Would you add a javadoc here too? Also, can we add some more per-segment information in the future? 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: 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
