[GitHub] [pinot] richardstartin commented on a change in pull request #8408: Combine Metadata and Dictionary based plans into single operator

[GitBox <gi...@apache.org>]

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



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/query/NonScanBasedAggregationOperator.java
##########
@@ -122,8 +131,26 @@ protected IntermediateResultsBlock getNextBlock() {
     return new IntermediateResultsBlock(_aggregationFunctions, aggregationResults, false);
   }
 
-  private double toDouble(Comparable value) {
-    if (value instanceof Number) {
+  private static Double getMinValue(DataSource dataSource) {
+    Dictionary dictionary = dataSource.getDictionary();
+    if (dictionary != null) {
+      return toDouble(dictionary.getMinVal());
+    }
+    return toDouble(dataSource.getDataSourceMetadata().getMinValue());

Review comment:
       it's nullable, so no. DataSources without dictionaries but with valid metadata qualify for use now. This cost of this check is kind of a 3rd or 4th order effect in the scope of a query.




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



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