Skip to content

fix: scope HashJoinExec build_time to hash-table construction only - #24271

Open
ulrichurriola-parada-ops wants to merge 1 commit into
apache:mainfrom
ulrichurriola-parada-ops:fix/hashjoin-elapsed-time
Open

fix: scope HashJoinExec build_time to hash-table construction only#24271
ulrichurriola-parada-ops wants to merge 1 commit into
apache:mainfrom
ulrichurriola-parada-ops:fix/hashjoin-elapsed-time

Conversation

@ulrichurriola-parada-ops

@ulrichurriola-parada-ops ulrichurriola-parada-ops commented Aug 11, 2026

Copy link
Copy Markdown

Rationale for this change

HashJoinExec's elapsed_compute metric (as reported by EXPLAIN ANALYZE and ExecutionPlanMetricsSet) includes time spent executing the entire build-side subtree, not just the join's own work. Any tool that sums elapsed_compute across a physical plan to estimate total CPU time (e.g. to build a cost/telemetry model) will double-count that subtree's compute: once under the child operator(s) that did the work, and again under HashJoinExec.

What changes are included in this PR?

build_time was previously timed around the entire collect_left_input future (via left_fut.get_shared(cx) in collect_build_side). That future both drains the build-side child's stream and builds the hash table, so the child's own compute got billed to build_time whenever the driving partition executed both phases inline.

  • Scoped the build_time timer inside collect_left_input to start only after the build-side stream has been fully folded, covering just the synchronous bounds/hash-map construction.
  • Removed the now-redundant wrapping timer in collect_build_side.

Are these changes tested?

Covered by the existing hash_join test suite (correctness of join output and other metrics is unaffected). No new test was added to assert the exact build_time/elapsed_compute value, since there isn't currently a way to attribute deterministic wall-clock cost to a specific subtree in a unit test.

Are there any user-facing changes?

elapsed_compute reported for HashJoinExec (e.g. via EXPLAIN ANALYZE) will be smaller and more accurate, reflecting only the join's own compute rather than including its build-side subtree. No public API changes.

build_time previously wrapped the whole collect_left_input future,
including the build-side child's own stream-draining work, so its
compute got double-counted into this join's elapsed_compute. Time only
the post-fold hash-map construction instead.
@github-actions github-actions Bot added the physical-plan Changes to the physical-plan crate label Aug 11, 2026
@ulrichurriola-parada-ops
ulrichurriola-parada-ops marked this pull request as ready for review August 11, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant