xiangfu0 opened a new pull request, #19655: URL: https://github.com/apache/pinot/pull/19655
Stacked on #19647 (aggregate call binding contract). Review only the top commit; the base updates when #19647 merges. ## Summary Carries `AggregateCallBinding`s through multi-stage planning and execution. **No aggregate requires a binding yet**, so plans, plan bytes and results are unchanged. Follow-ups opt in aggregate families one at a time. A binding describes the original SQL call. A final-stage aggregate may receive an intermediate OBJECT accumulator, so it cannot infer its result type from its input. It must keep the binding computed against the original input. This PR preserves it at every step: - **Stage splitting.** `PinotAggregateExchangeNodeInsertRule` and the physical-v2 `AggregatePushdownRule` bind each aggregate against its original input, before operands are replaced with accumulator references (`PinotRuleUtils.bindAggregateCall`). They carry the binding in a `BoundAggregationFunction`. A bound final type takes precedence over the legacy final-type override, including on server-final leaf stages. - **Plan nodes.** `RexExpression.FunctionCall` carries the binding, and `RexExpressionUtils` round-trips it to and from Calcite `AggregateCall`s. `RelToPlanNodeConverter`'s input-ref rewrites preserve it together with `isDistinct` and `ignoreNulls`; previously they dropped both flags. - **Plan serde.** The optional protobuf `FunctionCall.aggregationBinding` field is added in #19647. - **Leaf stages.** `CalciteRexExpressionParser` copies the binding onto the Thrift function of the single-stage leaf query. - **Intermediate and final stages.** `AggregateOperator` constructs the aggregate with the binding. `bindAggregateCall` returns `null` unless the aggregate's `AggregationFunctionType` requires a binding. Every aggregate therefore keeps its current `PinotSqlAggFunction` and return-type path until it opts in. ## Tests - `RexExpressionSerDeTest`: a bound aggregate call survives protobuf round-trip, keeping its OBJECT data type, and an unbound call stays unbound. - `CalciteRexExpressionParserTest`: a bound aggregate keeps its binding when converted to a leaf-stage Thrift function. - Existing tests pass unchanged: 43 planner test classes, including the plan-shape assertions in `ResourceBasedQueryPlansTest` and `QueryCompilationTest`, plus the runtime `AggregateOperatorTest` and `WindowAggregateOperatorTest`. In total, 1,814 tests with no failures. -- 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]
