gortiz commented on PR #18741:
URL: https://github.com/apache/pinot/pull/18741#issuecomment-5479396105
Closing this in favour of a stacked series, and replacing it rather than
shrinking it in place so
the review history here is not invalidated by a force-push that discards
most of the diff.
The problem was size, not direction. 7.2k lines in one pass is more than
anyone should be asked to
review, and @timothy-e's "it's harder to find and understand the logic
amidst all the plumbing" was
the right call — the statistics foundation alone has now been split three
ways, so the plumbing and
the logic are in separate PRs:
1. **contracts and stores** — what a statistic is, and the two `StatsStore`
implementations · #19409
2. **collection** — reading segment ZK metadata, and turning raw doc counts
into confidence-tiered
statistics · #19410
3. **selection and purge** — choosing a store by name, startup wiring,
orphan cleanup · #19411
4. **planner wiring** — expose statistics to Calcite (`getStatistic`,
metadata provider, cost model)
5. **join reordering** — the gated cost-based reorder phase
Each of the first three compiles and passes its tests on its own; none of
them changes a query plan.
Same content, reorganised: the branch was rebased onto current master and
the history rewritten so
each commit is a self-contained change rather than a change plus its later
fixups. #18740 has been
updated to match.
@timothy-e — on the independent fixes you asked to see moved out: the
quickstart fixes are no longer
in the foundation at all, they now ride with the join-reordering PR that
actually needed them for
its end-to-end run. Your question about lookup-join hints applies to that
same PR. The current
behaviour is the blunt one you would expect — any join carrying a hint
disables reordering for the
whole plan — and per-join veto is the obvious refinement. I will raise it
there explicitly rather
than let it get lost in the split.
@jadami10 — broker-side min/max pruning for the single-stage engine is
untouched by this
reorganisation; it still depends on the per-column statistics tier, which
comes after these.
--
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]