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

   ## Problem
   
   With the multi-stage engine and `usePhysicalOptimizer=true`, a no-GROUP-BY 
aggregate whose data is colocated on a single server is planned as a 
single-stage `AGGREGATE_DIRECT` leaf that returns **final** results 
(`SERVER_RETURN_FINAL_RESULT`). This happens, for example, when a query filters 
on a single partition value of a partitioned, `strictReplicaGroup` table so 
routing prunes to one server.
   
   For aggregation functions whose intermediate type differs from their final 
type — e.g. `DISTINCTCOUNTHLLPLUS` (`HyperLogLogPlus` → `LONG`), 
`DISTINCTCOUNT` (`Set` → `INT`) — the leaf crashes while serializing its output:
   
   ```
   SET useMultistageEngine=true; SET usePhysicalOptimizer=true;
   SELECT SUM(a), DISTINCTCOUNTHLLPLUS(user_id) FILTER (WHERE user_id <> '') 
FROM tbl WHERE part_col = 'x'
   
   Received 1 error from stage 1 on Server_...:
   class com.clearspring.analytics.stream.cardinality.HyperLogLogPlus cannot be 
cast to class java.lang.Long
   ```
   
   ## Root cause
   
   `AggregationResultsBlock` is inconsistent between the two methods the 
multi-stage leaf relies on:
   
   - `getDataSchema()` reports the **final** column types (e.g. `LONG`) when 
`isServerReturnFinalResult()` is true.
   - `getRows()` returned the **raw intermediate** `_results` (the 
`HyperLogLogPlus` object) without finalizing. Only `getDataTable()` finalized 
via `extractFinalResult()`.
   
   `LeafOperator.composeDirectMseBlock` consumes `getRows()` + 
`getDataSchema()`. Because the schema already claims the final type, no type 
conversion is applied, and the intermediate object is left in a column typed as 
its final type. It then fails on `MAILBOX_SEND` when the block is serialized.
   
   ## Fix
   
   `AggregationResultsBlock.getRows()` now finalizes via `extractFinalResult()` 
when `isServerReturnFinalResult()` is true, so the rows are consistent with the 
schema it already advertises. This mirrors `getDataTable()` and the group-by 
path, where `GroupByCombineOperator` already finalizes the indexed table for 
server-return-final.
   
   ## Scope
   
   - Only the **v2 physical optimizer** produces a no-GROUP-BY single-server 
`DIRECT` aggregate. The default (v1) multi-stage planner always splits 
no-GROUP-BY aggregates into LEAF + FINAL (intermediate `OBJECT` on the leaf, 
finalized in the FINAL stage), so it is **not** affected.
   - GROUP-BY `DIRECT` aggregates were already safe because 
`GroupByCombineOperator` finalizes the table.
   - This is a crash fix on an existing, option-gated path; prior behavior was 
a hard `ClassCastException`, so there is no behavior change for any query that 
previously succeeded.
   
   ## Testing
   
   Added `DirectAggregateObjectIntermediate.json` (a `replicated` table forces 
single-server colocation → `AGGREGATE_DIRECT`), covering:
   - `SUM` + `DISTINCTCOUNTHLLPLUS ... FILTER` (the reported shape),
   - plain `DISTINCTCOUNTHLLPLUS` and `DISTINCTCOUNT`,
   - a primitive-only case (intermediate type == final type) to confirm the 
finalize loop is a correct no-op,
   - a column-based-null-handling case with zero-match filters so a value 
finalizes to `NULL`.
   
   The legacy planner passes and the physical optimizer fails before / passes 
after the fix. The full `ResourceBasedQueriesTest` (3589 cases) and the 
relevant `pinot-core` aggregation tests are green.
   


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