scottlan0113 opened a new pull request, #19438:
URL: https://github.com/apache/pinot/pull/19438

   ## Motivation
   
   `CombinePlanNode.getCombineOperator()` builds the operator list and then 
picks the combine operator in one private method. An implementation that wants 
to substitute a single operator therefore has to reimplement the plan-node 
scheduling, tracing and termination checks above it.
   
   ## Change
   
   Move each operator's construction into its own protected method. Every 
condition and its ordering stays exactly where it was — only the construction 
moves, so behaviour is unchanged by construction rather than by argument.
   
   - Four non-streaming methods (`createAggregationCombineOperator`, 
`createGroupByCombineOperator`, `createSelectionCombineOperator`, 
`createDistinctCombineOperator`), followed by three streaming ones.
   - All four fields become `protected final` so subclasses can reach them.
   - The streaming methods read their flush threshold from the query context 
rather than taking it as a parameter, keeping every signature uniform.
   
   Streaming and non-streaming group-by have separate methods. They are 
different operators: the streaming one flushes partial results and relies on an 
aggregation above to merge them, so substituting one should not implicitly 
substitute the other.
   
   `createGroupByCombineOperator()` covers the 
`shouldSortAggregateUnderSafeTrim()` variants as well as the general operator. 
Its javadoc notes that an override must delegate that branch to `super` — those 
operators carry the safe-trim semantics the query was planned with, so silently 
replacing them would change results rather than fail.
   
   ## Testing
   
   51 existing tests pass across `CombinePlanNodeTest`, 
`SortedGroupByCombineOperatorsTest`, `SelectionCombineOperatorTest`, 
`StreamingGroupByCombineOperatorTest` and 
`StreamingDistinctCombineOperatorTest` — covering every branch touched.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to