yashmayya commented on PR #18941: URL: https://github.com/apache/pinot/pull/18941#issuecomment-4926967722
A deep audit of this default flip surfaced one more latent bug in the routing-query builder, fixed in 3af57d8abe — this one was a **silent wrong-results** risk, not just a planning failure. **Root cause**: `PlanNodeRoutingQueryBuilder` folded every Filter/Project it found in the leaf plan chain, silently skipping other node types without accounting for the row-space change. With `aggOptions(is_partitioned_by_group_by_keys='true')`, the aggregate is DIRECT (un-split, no exchange), so a HAVING filter lands in the same leaf fragment *above* the aggregate. Its InputRefs index the aggregate's output row space (`[col1, SUM(col3)]`), but the builder resolved them against the scan columns — producing a routing filter on an unrelated column — and then **overwrote** the genuine WHERE filter with it. If the mis-resolved column happened to be the table's time or partition column, segments containing matching rows could be pruned → silently wrong aggregates. Reproduced: `... WHERE col2 = 'x' GROUP BY col1 HAVING SUM(col3) > 10` produced routing filter `col2 > 10` instead of `col2 = 'x'`. **Fix**: both builders (`PlanNodeRoutingQueryBuilder` and its RelNode twin `LeafStageToPinotQuery`, which the physical-optimizer path uses) now stop folding at the first node that is neither Filter nor Project — a leaf boundary such as an un-split aggregate — since everything below the boundary is a genuine row-level condition and everything above operates on a different row space. Filters are also now AND-merged instead of overwritten. Strictly conservative: the change can only shrink the routing filter (less pruning), never extend it. **Audit coverage beyond this fix** (all verified clean): - All-pruned leaves (zero workers) across exchange shapes — plain select, global sort/limit, aggs, empty OVER(), UNION ALL, DISTINCT+LIMIT, and dynamic-broadcast semi-join with a fully-pruned build side — all plan successfully (new `testBrokerPruningAllPrunedLeafPlansAcrossExchangeShapes` locks this in with an all-pruned mock). - `TimeSegmentPruner`: NOT is a proper interval complement, unrecognized shapes keep-all, literal misparses throw → fail-open to unpruned. - Partition pruners: type-mismatched literals get a CAST on the column side → function-LHS → no pruning (safe direction). - Multi-input leaves (semi-join/lookup shapes) throw in the builder → caught → unfiltered fallback; runtime-injected dynamic IN filters never reach routing. New regression tests: mis-resolved-HAVING shape asserts the routing filter carries exactly the WHERE, plus the all-pruned shape sweep. -- 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]
