fix: avoid double-counting build_time in HashJoinExec elapsed_compute

[ulrichurriola-parada-ops]

Rationale for this change

HashJoinExec's elapsed_compute metric (visible via EXPLAIN ANALYZE, .metrics(), and any tooling built on top of them) overstates the actual compute time for the join, sometimes substantially (observed a +47% higher than build_time + join_time combined, even though elapsed_compute is documented/expected to equal exactly that sum). This misleads anyone using the metric to diagnose slow joins or to attribute CPU time within a query plan.

What changes are included in this PR?

BuildProbeJoinMetrics derives Clone and previously updated elapsed_compute from its own Drop impl. HashJoinExec::execute() clones this struct into the build-side future (collect_left_input) while the original is kept by HashJoinStream, so two instances exist for the lifetime of a join, and Drop fired once per instance — double-counting build_time whenever the build side yields control (e.g. returns Pending) at least once before completing, which is the common case for anything other than an already-materialized, single-batch build side.

This PR moves the elapsed_compute update into a new ElapsedComputeFinalizer, held behind an Arc on BuildProbeJoinMetrics. Cloning the outer struct now only bumps the Arc's reference count instead of creating a second independent Drop, so the update runs exactly once — when the last clone is dropped — using the fully-accumulated final values of build_time and join_time.

Are these changes tested?

Yes. Added build_probe_join_metrics_elapsed_compute_not_double_counted_on_clone, which reproduces the clone/drop sequence directly (without needing a full hash join) and asserts elapsed_compute == build_time + join_time exactly.

Ran the full existing joins:: test module (cargo test -p datafusion-physical-plan --lib joins::): 1023 passed, 0 failed, no regressions.

Are there any user-facing changes?

HashJoinExec's elapsed_compute metric will report lower (correct) values than before for any join whose build side isn't already fully materialized in a single poll — most visible in EXPLAIN ANALYZE output and any plan-visualization tooling built on DataFusion's metrics. No public API changes; BuildProbeJoinMetrics is a private (pub(crate)) type.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 91.42857% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.05%. Comparing base (70c26a0) to head (5b7cb26).
⚠️ Report is 45 commits behind head on main.

Files with missing lines	Patch %	Lines
datafusion/physical-plan/src/joins/utils.rs	91.42%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #24142    +/-   ##
========================================
  Coverage   81.04%   81.05%            
========================================
  Files        1105     1106     +1     
  Lines      380163   380591   +428     
  Branches   380163   380591   +428     
========================================
+ Hits       308111   308495   +384     
- Misses      53834    53874    +40     
- Partials    18218    18222     +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kosiew]

@ulrichurriola-parada-ops,

Thanks for working on this. The change looks good to me. Moving elapsed_compute finalization behind a shared Arc addresses the double-counting issue cleanly, and the regression test does a good job of covering the ownership behavior.

I left one non-blocking suggestion for an additional execution-level regression test that could make this even more robust.

[kosiew]

Nice regression test. One additional test that might be worth considering is an execution-level HashJoinExec test where the build stream yields before completing. This unit test covers the shared-finalizer ownership behavior well, while an execution-level test would also exercise the HashJoinExec::execute clone and future wiring that originally exposed the issue. Not blocking for this PR.

[ulrichurriola-parada-ops]

Thank you for the review,I'll make a follow-up for the additional test.