Skip to content

Improve test coverage for join operator metrics #25333

Description

@CuteChuanChuan

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_rows of SymmetricHashJoinExec are not checked by any test
  • build_input_rows of HashJoinExec is not checked by any Rust test, only through EXPLAIN ANALYZE snapshots in .slt files

Describe the solution you'd like

Add metric tests for the join operators, one operator per PR:

  • HashJoinExec
  • NestedLoopJoinExec
  • SortMergeJoinExec
  • SymmetricHashJoinExec
  • CrossJoinExec
  • PiecewiseMergeJoinExec
  • AsOfJoinExec

For each operator:

  1. Check which of its metrics are already covered by existing tests.
  2. Add tests with multi-batch inputs and small batch_size values, asserting metrics whose values should not depend on how the input is batched, such as row counts and ratio metrics like probe_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 ANALYZE cases in .slt (with set datafusion.execution.batch_size = ...) instead of Rust unit tests on MetricsSet. 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_size values 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 as output_batches.

Which style would maintainers prefer for this kind of test?

Additional context

Related: #25077, #25272

Activity

  1. rgbuilds commented on Sep 19, 2026

    @rgbuilds
    Contributor

    I’d like to work on the CrossJoinExec portion of this issue. I plan to add Rust test coverage using multi-batch inputs and assert batching-independent metrics such as build_input_rows, input_rows, and output_rows. I’ll start working on a PR—please let me know if you see any concerns with this approach.

  2. ArturoAHR commented on Sep 20, 2026

    @ArturoAHR

    I plan to take on the remaining operators, I'll be assigning myself. @rgbuilds seems to already have tackled CrossJoinExec and 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.

  3. ArturoAHR commented on Sep 20, 2026

    @ArturoAHR

    take

  4. ArturoAHR commented on Oct 25, 2026

    @ArturoAHR

    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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions