siddharthteotia commented on a change in pull request #7830: URL: https://github.com/apache/pinot/pull/7830#discussion_r764484966
########## File path: pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java ########## @@ -62,57 +68,31 @@ public AggregationPlanNode(IndexSegment indexSegment, QueryContext queryContext) public Operator<IntermediateResultsBlock> run() { assert _queryContext.getAggregationFunctions() != null; - int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs(); - AggregationFunction[] aggregationFunctions = _queryContext.getAggregationFunctions(); + boolean hasFilteredPredicates = _queryContext.getFilteredAggregationContexts() + .size() != 0; - FilterPlanNode filterPlanNode = new FilterPlanNode(_indexSegment, _queryContext); - BaseFilterOperator filterOperator = filterPlanNode.run(); + Pair<Pair<BaseFilterOperator, TransformOperator>, + BaseOperator<IntermediateResultsBlock>> pair = + buildOperators(_queryContext.getFilter(), false); - // Use metadata/dictionary to solve the query if possible - // TODO: Use the same operator for both of them so that COUNT(*), MAX(col) can be optimized - if (filterOperator.isResultMatchingAll()) { - if (isFitForMetadataBasedPlan(aggregationFunctions)) { - return new MetadataBasedAggregationOperator(aggregationFunctions, _indexSegment.getSegmentMetadata(), - Collections.emptyMap()); - } else if (isFitForDictionaryBasedPlan(aggregationFunctions, _indexSegment)) { - Map<String, Dictionary> dictionaryMap = new HashMap<>(); - for (AggregationFunction aggregationFunction : aggregationFunctions) { - String column = ((ExpressionContext) aggregationFunction.getInputExpressions().get(0)).getIdentifier(); - dictionaryMap.computeIfAbsent(column, k -> _indexSegment.getDataSource(k).getDictionary()); - } - return new DictionaryBasedAggregationOperator(aggregationFunctions, dictionaryMap, numTotalDocs); - } - } + if (hasFilteredPredicates) { + int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs(); + AggregationFunction[] aggregationFunctions = _queryContext.getAggregationFunctions(); - // Use star-tree to solve the query if possible - List<StarTreeV2> starTrees = _indexSegment.getStarTrees(); - if (starTrees != null && !StarTreeUtils.isStarTreeDisabled(_queryContext)) { - AggregationFunctionColumnPair[] aggregationFunctionColumnPairs = - StarTreeUtils.extractAggregationFunctionPairs(aggregationFunctions); - if (aggregationFunctionColumnPairs != null) { - Map<String, List<CompositePredicateEvaluator>> predicateEvaluatorsMap = - StarTreeUtils.extractPredicateEvaluatorsMap(_indexSegment, _queryContext.getFilter(), - filterPlanNode.getPredicateEvaluatorMap()); - if (predicateEvaluatorsMap != null) { - for (StarTreeV2 starTreeV2 : starTrees) { - if (StarTreeUtils.isFitForStarTree(starTreeV2.getMetadata(), aggregationFunctionColumnPairs, null, - predicateEvaluatorsMap.keySet())) { - TransformOperator transformOperator = - new StarTreeTransformPlanNode(starTreeV2, aggregationFunctionColumnPairs, null, - predicateEvaluatorsMap, _queryContext.getDebugOptions()).run(); - return new AggregationOperator(aggregationFunctions, transformOperator, numTotalDocs, true); - } - } - } - } + Set<ExpressionContext> expressionsToTransform = + AggregationFunctionUtils.collectExpressionsToTransform(aggregationFunctions, null); + BaseOperator<IntermediateResultsBlock> aggregationOperator = pair.getRight(); + + assert pair != null && aggregationOperator != null; + + //TODO: For non star tree, non filtered aggregation, share the main filter transform operator Review comment: I suggest making this code a bit more readable. I find it super confusing because of all the Pair logic. Comments and descriptive variable names would be helpful -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org