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: [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]
