nikolas-sapa commented on PR #26246:
URL: https://github.com/apache/datafusion/pull/26246#issuecomment-6245982425
Verified every documented metric against `apache/datafusion@main` source
(not just reformatted). All 11 metric names exist and are actually emitted by
`NestedLoopJoinExec`. No changes were needed.
**Build-probe metrics** — `NestedLoopJoinMetrics.join_metrics` is a
`BuildProbeJoinMetrics`
(`datafusion/physical-plan/src/joins/utils.rs:1866-1893`), and each name is
emitted by `NestedLoopJoinExec`:
| Metric | Verified against |
| --- | --- |
| `build_time` | `subset_time("build_time")` `utils.rs:1872`; incremented in
left-side collection `nested_loop_join.rs:1222,1260` |
| `build_input_batches` | `counter("build_input_batches")` `utils.rs:1875`;
incremented `nested_loop_join.rs:1227` |
| `build_input_rows` | `counter("build_input_rows")` `utils.rs:1879`;
incremented `nested_loop_join.rs:1228` |
| `build_mem_used` | `peak_memory_usage("build_mem_used")` `utils.rs:1884`;
a `Gauge` (`utils.rs:1814`), incremented `nested_loop_join.rs:1226,1295` —
hence "Peak" in the description |
| `join_time` | `subset_time("join_time")` `utils.rs:1870`; incremented
`nested_loop_join.rs:2362,2848` |
| `input_batches` | `counter("input_batches")` `utils.rs:1887`; incremented
`nested_loop_join.rs:2856` |
| `input_rows` | `counter("input_rows")` `utils.rs:1891`; incremented
`nested_loop_join.rs:2855` |
**`selectivity`** — declared `MetricType::Summary` via
`ratio_metrics("selectivity")` at `nested_loop_join.rs:2237-2239`;
numerator/denominator confirmed as `add_part(output_rows)` (`:3755`) over
`add_total(left_rows * right_rows)` (`:2927`), matching the documented formula.
**Spill metrics** — `NestedLoopJoinMetrics.spill_metrics` is `SpillMetrics`
(`nested_loop_join.rs:2240`). Worth noting the emitted *display names* differ
from the Rust struct fields: the field is `spill_file_count`, but it is
registered through `MetricBuilder::spill_count(...)` (`baseline.rs:290`) and
its display name is `spill_count` (`value.rs:828`). The docs correctly use the
emitted names `spill_count` / `spilled_bytes` / `spilled_rows`
(`value.rs:828-832`).
**Claims checked as well as names:**
- "buffers the left input" — confirmed; left-side collection is what
increments `build_time`/`build_input_rows`, so left *is* the build side.
- "`elapsed_compute` is the sum of build-side collection and subsequent join
processing" — confirmed by `ElapsedComputeFinalizer::drop`, which adds
`build_time` + `join_time` into `baseline.elapsed_compute()`
(`utils.rs:1859-1864`). Mirrors the existing `HashJoinExec` wording.
- "`selectivity` is Summary, the rest are Dev" — confirmed;
`MetricBuilder::new` defaults to `MetricType::Dev` (`builder.rs:82`), and only
`selectivity` overrides to Summary.
- `probe_hit_rate` / `avg_fanout` are registered by `BuildProbeJoinMetrics`
but are correctly **omitted** here: `grep` over `nested_loop_join.rs` shows
`NestedLoopJoinExec` never increments them, and they describe equijoin-key
behaviour that NLJ does not have.
**Formatting:** both new tables are prettier-aligned (uniform line widths of
105 and 140 chars) and match the surrounding `HashJoinExec` / `AggregateExec`
entries. New `### NestedLoopJoinExec` section is placed between `HashJoinExec`
and `AggregateExec`, adjacent to the other join operator.
--
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]
