mayankshriv commented on a change in pull request #6285:
URL: https://github.com/apache/incubator-pinot/pull/6285#discussion_r533563925
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctAggregationFunction.java
##########
@@ -18,31 +18,25 @@
*/
package org.apache.pinot.core.query.aggregation.function;
-import com.google.common.base.Preconditions;
-import java.util.Arrays;
import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
import org.apache.pinot.common.function.AggregationFunctionType;
-import org.apache.pinot.common.utils.DataSchema;
import org.apache.pinot.common.utils.DataSchema.ColumnDataType;
import org.apache.pinot.core.common.BlockValSet;
-import org.apache.pinot.core.common.RowBasedBlockValueFetcher;
-import org.apache.pinot.core.data.table.Record;
import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
-import org.apache.pinot.core.query.aggregation.ObjectAggregationResultHolder;
-import
org.apache.pinot.core.query.aggregation.function.customobject.DistinctTable;
import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
import org.apache.pinot.core.query.request.context.ExpressionContext;
import org.apache.pinot.core.query.request.context.OrderByExpressionContext;
/**
- * The DISTINCT clause in SQL is executed as the DISTINCT aggregation function.
- * TODO: Support group-by
+ * The DISTINCT clause in SQL is represented as the DISTINCT aggregation
function. Currently it is only used to wrap the
+ * information for the distinct queries.
+ * TODO: Use a separate way to represent DISTINCT instead of aggregation.
*/
@SuppressWarnings("rawtypes")
-public class DistinctAggregationFunction implements
AggregationFunction<DistinctTable, Comparable> {
+public class DistinctAggregationFunction implements
AggregationFunction<Object, Comparable> {
Review comment:
Seems this class does not implement any of the methods?
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/plan/maker/InstancePlanMakerImplV2.java
##########
@@ -123,8 +124,13 @@ public Plan makeInstancePlan(List<IndexSegment>
indexSegments, QueryContext quer
@Override
public PlanNode makeSegmentPlanNode(IndexSegment indexSegment, QueryContext
queryContext) {
- if (QueryContextUtils.isAggregationQuery(queryContext)) {
- // Aggregation query
+ if (QueryContextUtils.isSelectionQuery(queryContext)) {
Review comment:
Same here, common case first?
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/plan/CombinePlanNode.java
##########
@@ -172,7 +173,19 @@ public CombinePlanNode(List<PlanNode> planNodes,
QueryContext queryContext, Exec
return new StreamingSelectionOnlyCombineOperator(operators,
_queryContext, _executorService, _endTimeMs,
_streamObserver);
}
- if (QueryContextUtils.isAggregationQuery(_queryContext)) {
+ if (QueryContextUtils.isSelectionQuery(_queryContext)) {
Review comment:
Common case (of aggr/group-by) should be first?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
