xiangfu0 commented on code in PR #20035:
URL: https://github.com/apache/pinot/pull/20035#discussion_r4396255112
##########
pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java:
##########
@@ -353,6 +355,37 @@ public Map<ExpressionContext, ExpressionContext>
getExpressionOverrideHints() {
return _expressionOverrideHints;
}
+ /// Prepares derived execution state after logical expressions change,
before fetch or segment planning.
+ /// Reuses compiled aggregations whose input trees did not change. The
supplied set uses function identity because
+ /// expression rewrites can mutate function arguments, including their hash
codes, in place.
+ public void prepareForExecution(Set<FunctionContext> changedFunctions) {
+ int numAggregations = _aggregationFunctionContexts.size();
Review Comment:
Verified this invariant at `62abc9a0e2`: `QueryContext` has a private
constructor, and its only construction site is `Builder.build()`. The builder
unconditionally extracts and stores the original aggregation contexts before
the first `prepareForExecution(...)` call; extraction creates a non-null list
even for queries without aggregations. The instance planner subsequently
receives that fully built context.
I am retaining this initialization contract. A null-to-empty fallback could
conceal an incompletely constructed context and omit compiled aggregations. No
source change is needed for the current call paths.
--
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]
