You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "gortiz (via GitHub)" <gi...@apache.org> on 2026/09/02 14:58:39 UTC
[PR] Report the optimizer's row-count estimates alongside the actual ones in query stats [pinot]
gortiz opened a new pull request, #19435:
URL: https://github.com/apache/pinot/pull/19435
## Motivation
The multi-stage engine already reports how many rows each operator emitted, and the planner already
knows how many it expected. The two have never met, so there is no ordinary way to answer "was the
estimate for this query any good?".
Today the only route is `EXPLAIN PLAN INCLUDING ALL ATTRIBUTES` — itself obscure, since the default
explain level hides row counts — followed by working out by hand whether the numbers are plausible.
That does not scale past one query at a time, and it cannot be done at all for a query that has
already run.
Estimate quality is worth being able to see. A plan alone cannot tell you whether the optimizer
chose well from good information or badly from bad information: a join whose true result is a
million rows and one estimated at a hundred billion produce identical-looking plans.
## Change
`SET includeEstimatedRows=true` adds an `estimatedRows` field next to the actual count on each node
of the response's `stageStats`:
```json
{ "type": "HASH_JOIN", "planNodeIds": [4],
"emittedRows": 6001215, "estimatedRows": 108021870000, ... }
```
Off by default. The estimates are captured while converting `RelNode`s to `PlanNode`s — the only
point where both representations exist — carried on `SubPlan`, handed to `DispatchableSubPlan` by
the dispatch planner, and looked up by the stats renderers against the same plan nodes the runtime
already reports against.
A query option rather than a config, because the question is per query: it has to be answerable for
the query that was actually slow, without a restart or a log-level change.
## This needs no statistics to be useful
With no statistics provider configured, Calcite still supplies its own row-count estimates, so
estimate-versus-actual works on any broker today. When table statistics are available the same
field starts reporting statistics-backed numbers instead. Nothing here depends on cost-based
optimization being enabled.
## What you will see first, and why it is not a bug
On any join query the divergence will be enormous. Only scan row counts are statistics-backed;
every join and filter selectivity is currently Calcite's constant `RelMdUtil.guessSelectivity`,
0.15 per equality conjunct. Measured on TPC-H SF1, matching the plan to every digit:
```
lineitem ⋈ partsupp 6,001,215 × 800,000 × 0.15² = 1.0802187e11
⋈ supplier 1.0802187e11 × 10,000 × 0.15 = 1.62032805e14
⋈ nation 1.62032805e14 × 25 × 0.15² = 9.1143452e13
```
The first join's true cardinality is about 6M, so the estimate is ~18,000× high, and a
`n_name = 'JAPAN'` filter is estimated at 0.15 rather than 1/25.
That is this change working correctly. It surfaces a known gap — join selectivity is not yet
NDV-based — rather than introducing one, and being able to see that gap is the point.
## Design notes
**Nothing is added to `PlanNode`.** `PlanNode` is serialized to servers, so a field there would be
a wire-format and mixed-version concern for what is purely a broker-side diagnostic. The estimates
live in a broker-side map; servers are unaffected and need no upgrade.
**Identity-keyed, not equals-keyed.** Two structurally identical nodes are distinct plan nodes with
potentially different estimates, and equals-keying would silently merge them. The keys stay valid
downstream because plan fragmentation wraps the converted nodes in new mailbox nodes rather than
copying them — there is a test that fails loudly if that ever changes.
**Both renderers annotate.** A query takes either the streaming path (`MultiStageStatsTreeBuilder`)
or the plan-visiting one (`InStageStatsTreeBuilder`). Wiring only the first would make the field
appear for some queries and silently vanish for others, which is worse than not reporting it.
**Attribution for fused nodes is explicit.** A stats node can cover several plan nodes: operators
fuse, and a leaf stage compiles its whole pushed-down subtree into a single operator with one
emitted-row count. The estimate reported for such a group is the topmost node's, because that is
the node whose output those emitted rows represent. Consequently there are no scan-level actuals
inside a leaf stage — a property of the engine, not of this change.
**A missing estimate renders no field**, rather than a zero, which a consumer would read as "the
optimizer predicted no rows". Failure to obtain an estimate is logged at DEBUG and left
unrecorded: a diagnostic must never break planning.
**No new constructor parameters where they would pile up.** `DispatchableSubPlan` already takes
six, so the estimates are set once by the dispatch planner instead of becoming a seventh; and the
broker passes the map — empty when the option is off — rather than an extra boolean, so "not
requested" and "nothing recorded" are one code path.
## Testing
- `RelToPlanNodeConverterTest`: every node in a converted tree carries an estimate, and the map is
identity-keyed.
- `QueryCompilationTest`: the estimates survive the whole planning chain (converter → `SubPlan` →
dispatch planner → `DispatchableSubPlan`), every hop being somewhere they could be dropped while
the unit tests either side still passed; that a real plan yields positive estimates; that plan
node identity is preserved through fragmentation; and that estimates are captured with no
statistics provider at all.
- `MultiStageStatsTreeBuilderTest`: the estimate lands on the node whose actual count it should be
compared against, with the actual untouched beside it; it is absent when not requested; a fused
node reports the topmost estimate; a node with no estimate renders no field; and both renderers
behave identically.
## Possible follow-ups
- **Estimates in the `EXPLAIN` text**, which is the more legible format for a human. A small
`RelWriter` subclass does it. Note for whoever picks this up: the default explain level should
*not* be moved to `ALL_ATTRIBUTES` to achieve it — that level also emits each node's internal id,
which is assigned per planning run, so it makes explain output non-deterministic and rewrites
thousands of pinned plan lines. Appending one attribute at the caller's existing level avoids
both.
- **`EXPLAIN ANALYZE`**, reporting the same pair of numbers over the plan tree rather than over the
stats tree. `PhysicalExplainPlanVisitor` already renders from plan nodes, so the annotation is
small; the real work is that `EXPLAIN` does not execute the query today.
## Release notes
Adds the `includeEstimatedRows` query option (default off). No behavior change when unset.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Report the optimizer's row-count estimates alongside the actual ones in query stats [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19435:
URL: https://github.com/apache/pinot/pull/19435#issuecomment-5512744797
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19435?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 100.00%. Comparing base ([`bb44811`](https://app.codecov.io/gh/apache/pinot/commit/bb448114ab2b93ddc2e27d53ec0ef1b4dee73d9d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`c7d3dfc`](https://app.codecov.io/gh/apache/pinot/commit/c7d3dfc51285603d2c0cca3daeb45184893edfa6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19435 +/- ##
==============================================
+ Coverage 67.59% 100.00% +32.40%
+ Complexity 1430 6 -1424
==============================================
Files 3487 3 -3484
Lines 224320 6 -224314
Branches 35417 0 -35417
==============================================
- Hits 151623 6 -151617
+ Misses 60679 0 -60679
+ Partials 12018 0 -12018
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+32.40%)` | :arrow_up: |
| [lane-a](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [lane-b](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (+32.40%)` | :arrow_up: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19435?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org