walterddr commented on a change in pull request #8408:
URL: https://github.com/apache/pinot/pull/8408#discussion_r835327232



##########
File path: 
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
##########
@@ -64,6 +64,10 @@
           DISTINCTCOUNTHLLMV, DISTINCTCOUNTRAWHLL, DISTINCTCOUNTRAWHLLMV, 
SEGMENTPARTITIONEDDISTINCTCOUNT,
           DISTINCTCOUNTSMARTHLL);
 
+  // DISTINCTCOUNT excluded because consuming segment metadata contains 
unknown cardinality when there is no dictionary

Review comment:
       wondering if we can somehow add to the logic of 
`isFitForMetadataBasePlan` checker for distinct count. basically one additional 
check to see if it is dictionary encoded?
   we already did additional check for COUNT anyway. good to add DISTINCTCOUNT 
too?

##########
File path: 
pinot-core/src/test/java/org/apache/pinot/queries/ExplainPlanQueriesTest.java
##########
@@ -505,7 +505,7 @@ public void testSelectAggregate() {
     result1.add(new Object[]{"AGGREGATE_METADATA", 2, 1});
     check(query1, new ResultTable(DATA_SCHEMA, result1));
 
-    String query2 = "EXPLAIN PLAN FOR SELECT min(invertedIndexCol1) FROM 
testTable";
+    String query2 = "EXPLAIN PLAN FOR SELECT 
distinctcounthll(invertedIndexCol1) FROM testTable";

Review comment:
       why change the query to be tested? not the explained plan node below?




-- 
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

Reply via email to