adasari commented on code in PR #18334:
URL: https://github.com/apache/pinot/pull/18334#discussion_r3639425675
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -115,17 +122,34 @@ public Operator<AggregationResultsBlock>
buildNonFilteredAggOperator() {
boolean hasNullValues = _queryContext.isNullHandlingEnabled() &&
hasNullValues(aggregationFunctions);
if (!hasNullValues) {
- // Priority 2: Check if non-scan based aggregation is feasible
- if (filterOperator.isResultMatchingAll() && isFitForNonScanBasedPlan()) {
+ // when the filter matches all documents, resolve as many functions as
possible from the column
+ // dictionary/metadata without scanning the segment. Eligibility is
evaluated once per function here
+ // and reused for both the fully non-scan path (all functions
resolvable) and
+ // the partial path (some functions resolvable).
+ if (filterOperator.isResultMatchingAll()) {
+ boolean[] metadataResolvable = new
boolean[aggregationFunctions.length];
DataSource[] dataSources = new DataSource[aggregationFunctions.length];
+ int numResolved = 0;
for (int i = 0; i < aggregationFunctions.length; i++) {
- List<?> inputExpressions =
aggregationFunctions[i].getInputExpressions();
- if (!inputExpressions.isEmpty()) {
- String column = ((ExpressionContext)
inputExpressions.get(0)).getIdentifier();
- dataSources[i] = _indexSegment.getDataSource(column,
_queryContext.getSchema());
+ DataSource dataSource =
getDataSourceForAggregationFunction(aggregationFunctions[i]);
+ if (isFitForNonScanBasedPlan(aggregationFunctions[i], dataSource)) {
+ metadataResolvable[i] = true;
+ dataSources[i] = dataSource;
+ numResolved++;
}
}
- return new NonScanBasedAggregationOperator(_queryContext, dataSources,
numTotalDocs);
+
+ if (numResolved == aggregationFunctions.length) {
+ // Priority 2: all functions can be resolved from
dictionary/metadata -> fully non-scan based execution
+ return new NonScanBasedAggregationOperator(_queryContext,
dataSources, numTotalDocs);
+ }
+ if (numResolved > 0) {
+ // some functions can be resolved from dictionary/metadata; the rest
fall back to scan-based
+ // execution in the AggregationOperator.
+ aggregationInfo =
AggregationFunctionUtils.buildAggregationInfoWithoutStarTree(_segmentContext,
_queryContext,
Review Comment:
thanks for suggestion. will look into it as a follow-up.
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -173,40 +197,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
return false;
}
- return true;
+
+ // MIN/MAX/MINMAXRANGE derive their result numerically from the column
min/max, which is only valid for numeric
+ // columns. Non-numeric columns (e.g. BYTES) store min/max as raw values
that cannot be parsed as numbers.
+ if (NUMERIC_METADATA_FUNCTIONS.contains(functionType)
+ &&
!dataSource.getDataSourceMetadata().getDataType().getStoredType().isNumeric()) {
+ return false;
+ }
+
+ if (dataSource.getDictionary() != null &&
DICTIONARY_BASED_FUNCTIONS.contains(functionType)) {
+ return true;
+ }
+
+ return METADATA_BASED_FUNCTIONS.contains(functionType)
+ && dataSource.getDataSourceMetadata().getMaxValue() != null
+ && dataSource.getDataSourceMetadata().getMinValue() != null;
}
private static boolean canOptimizeFilteredCount(BaseFilterOperator
filterOperator,
AggregationFunction[] aggregationFunctions) {
return (aggregationFunctions.length == 1 &&
aggregationFunctions[0].getType() == COUNT)
&& filterOperator.canOptimizeCount();
}
+
+ /**
+ * Returns the data source for the given aggregation function's argument, or
{@code null} if the function has no
+ * argument (e.g. {@code COUNT(*)}) or its argument is not a single column
identifier (e.g. {@code COUNT(1)} or a
+ * transform expression), in which case it cannot be resolved from
dictionary/metadata.
+ *
+ * @param aggregationFunction aggregation function whose argument data
source is resolved
+ * @return the argument's data source, or {@code null} if it has no single
identifier argument
+ */
+ @Nullable
+ private DataSource
getDataSourceForAggregationFunction(AggregationFunction<?, ?>
aggregationFunction) {
+ List<?> inputExpressions = aggregationFunction.getInputExpressions();
+ if (!inputExpressions.isEmpty()) {
+ ExpressionContext argument =
aggregationFunction.getInputExpressions().get(0);
Review Comment:
Done.
--
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]
