xiangfu0 commented on PR #19264: URL: https://github.com/apache/pinot/pull/19264#issuecomment-5349281629
Pushed ad2b4ba fixing a trimming bug in the parallel derive that this design surfaced. **Bug:** the derive built its output table with `resultSize = numGroupsLimit`. For a grouping-set query *without* ORDER BY, the concurrent upsert caps at that size and drops brand-new keys once full — so which derived groups survived depended on thread interleaving (non-deterministic), and an entire low-magnitude grouping set (e.g. the grand total) could be starved if the detail set filled the quota first. **Fix:** the derive is a bounded transformation of the already-bounded base groups (base is capped at `numGroupsLimit` per segment, so derived ≤ `numGroupsLimit × numSets`), so it must not re-apply that per-segment guardrail. The derived table is now built with an unbounded result size; the real ORDER BY + LIMIT is deferred to the broker, and per-segment memory is still bounded upstream by the base-grouping `numGroupsLimit` cap. In other words: trimming is disabled on the derive and only the broker trims — consistent with the existing "grouping-set queries must not trim per server" rule. Added a regression test (`DeriveGroupingSetsTest`) that sets a small `numGroupsLimit`, then asserts the grand-total set is never dropped and that parallel derive is deterministic across runs. It fails before the fix (non-deterministic output) and passes after. All 75 grouping-sets integration tests still pass. -- 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]
