Repository navigation
Description
Is your feature request related to a problem or challenge?
Working on #25272 (issue: #25077) showed that join metrics can be wrong without any test failing. probe_hit_rate and avg_fanout of HashJoinExec were reported too low whenever a probe batch was split into several chunks. Before that PR, their values were only checked through EXPLAIN ANALYZE snapshots in .slt files (a Rust test only checked that they appear), on inputs small enough that no probe batch is ever split into chunks, and the Rust tests that do run with small batch_size values did not assert them.
Other join metrics also have little or no test coverage, for example:
left_input_rows/right_input_rowsofSymmetricHashJoinExecare not checked by any testbuild_input_rowsofHashJoinExecis not checked by any Rust test, only throughEXPLAIN ANALYZEsnapshots in.sltfiles
Describe the solution you'd like
Add metric tests for the join operators, one operator per PR:
HashJoinExecNestedLoopJoinExecSortMergeJoinExecSymmetricHashJoinExecCrossJoinExecPiecewiseMergeJoinExecAsOfJoinExec
For each operator:
- Check which of its metrics are already covered by existing tests.
- Add tests with multi-batch inputs and small
batch_sizevalues, asserting metrics whose values should not depend on how the input is batched, such as row counts and ratio metrics likeprobe_hit_rate.
The tests added in #25272 (join_probe_metrics_count_each_probe_row_once and join_probe_metrics_count_probe_row_starting_new_chunk) follow this approach for HashJoinExec, using the existing hash_join_exec_configs rstest matrix.
If this sounds reasonable, I can turn this into a tracking issue with one sub-task per operator.
Describe alternatives you've considered
EXPLAIN ANALYZEcases in.slt(withset datafusion.execution.batch_size = ...) instead of Rust unit tests onMetricsSet. These are closer to what users see, but non-deterministic values such as timings need<slt:ignore>, and running the same query over a matrix of configurations is less convenient.- A shared helper that runs any join plan at several
batch_sizevalues and compares metrics across runs, instead of per-operator tests. Less code per operator, but harder to express metrics that legitimately depend on batching, such asoutput_batches.
Which style would maintainers prefer for this kind of test?
Activity
rgbuilds commentedon Sep 19, 2026
I’d like to work on the
CrossJoinExecportion of this issue. I plan to add Rust test coverage using multi-batch inputs and assert batching-independent metrics such asbuild_input_rows,input_rows, andoutput_rows. I’ll start working on a PR—please let me know if you see any concerns with this approach.ArturoAHR commentedon Sep 20, 2026
I plan to take on the remaining operators, I'll be assigning myself. @rgbuilds seems to already have tackled
CrossJoinExecand submitted a draft PR at the time of writing this. Seems like a good first issue to get understanding of the project so I can hit the ground running for #25085.ArturoAHR commentedon Sep 20, 2026
take
test: cover CrossJoinExec row metrics across batched inputs
test: cover CrossJoinExec row metrics across batched inputs
test: cover CrossJoinExec row metrics across batched inputs
test: cover CrossJoinExec row metrics across batched inputs
test: cover CrossJoinExec row metrics across batched inputs (apache#2…
test: cover CrossJoinExec row metrics across batched inputs (apache#2…
ArturoAHR commentedon Oct 25, 2026
Still working on this, it's been a bit hard to get into the project, but I think I should have a PR up soon enough with Hash Join tests!