adasari commented on code in PR #18334:
URL: https://github.com/apache/pinot/pull/18334#discussion_r3694715348
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -173,40 +204,68 @@ private boolean hasNullValues(AggregationFunction[]
aggregationFunctions) {
}
/**
- * Returns {@code true} if the given aggregations can be solved with
dictionary or column metadata, {@code false}
- * otherwise.
+ * Returns {@code true} if the given aggregation function can be resolved
from the column dictionary or metadata
+ * (without scanning the segment), {@code false} otherwise. {@code COUNT} is
always eligible. Functions whose result
+ * is derived numerically from the column min/max (e.g. MIN, MAX,
MINMAXRANGE) are only eligible for numeric columns,
+ * since non-numeric columns (e.g. BYTES) store min/max as raw values that
cannot be parsed as numbers.
+ *
+ * @param aggregationFunction aggregation function to test
+ * @param dataSource the function argument's data source (see {@link
#getDataSourceForAggregationFunction})
*/
- private boolean isFitForNonScanBasedPlan() {
- AggregationFunction[] aggregationFunctions =
_queryContext.getAggregationFunctions();
- assert aggregationFunctions != null;
- for (AggregationFunction<?, ?> aggregationFunction : aggregationFunctions)
{
- if (aggregationFunction.getType() == COUNT) {
- continue;
- }
- ExpressionContext argument =
aggregationFunction.getInputExpressions().get(0);
- if (argument.getType() != ExpressionContext.Type.IDENTIFIER) {
- return false;
- }
- DataSource dataSource =
_indexSegment.getDataSource(argument.getIdentifier(),
_queryContext.getSchema());
- if (DICTIONARY_BASED_FUNCTIONS.contains(aggregationFunction.getType())) {
- if (dataSource.getDictionary() != null) {
- continue;
- }
- }
- if (METADATA_BASED_FUNCTIONS.contains(aggregationFunction.getType())) {
- if (dataSource.getDataSourceMetadata().getMaxValue() != null
- && dataSource.getDataSourceMetadata().getMinValue() != null) {
- continue;
- }
- }
+ private boolean isFitForNonScanBasedPlan(AggregationFunction<?, ?>
aggregationFunction,
+ @Nullable DataSource dataSource) {
+ AggregationFunctionType functionType = aggregationFunction.getType();
+ if (functionType == COUNT) {
+ return true;
+ }
+
+ if (dataSource == null) {
+ // Aggregation function does not have a single identifier argument (e.g.
COUNT(*) or COUNT(1)),
+ // so it cannot be resolved from metadata
Review Comment:
comment is misleading. Handing null dataSource condition there for
https://github.com/apache/pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java#L116-L120.
Updated comment.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
