Skip to content

Stop reading the right stream when the hash table is empty - #23

Merged
rubensworks merged 1 commit into
masterfrom
claude/hash-join-empty-build-shortcircuit
Sep 7, 2026
Merged

rubensworks merged 1 commit into
masterfrom
claude/hash-join-empty-build-shortcircuit

Conversation

@rubensworks

@rubensworks rubensworks commented Sep 7, 2026 •

Copy link
Copy Markdown
Member

An inner hash join over an empty hash table produces nothing, so the right stream never has to be read. HashJoin read it in full regardless: once the left stream ends, allowJoining unconditionally starts pulling from the right and hashing every item against an empty map.

That is cheap for an in-memory iterator, but not for one backed by a remote source.

Impact

Found while investigating a performance regression in Comunica's WatDiv Triple Pattern Fragments benchmark. Three of the queries return no results, and the plan puts an empty intermediate on the build side of a hash join. Comunica then downloaded every page of the remaining fragments to produce nothing.

Measured against the WatDiv-10 dataset used in Comunica's CI, with a local LDF server, 5 query instances per template, HTTP requests summed:

query before after
S3 335 35
S5 140 35

All 20 WatDiv templates return identical results, and the other 18 issue an identical number of requests.

Why the iterator is ended from read

The obvious implementation, ending as soon as the left stream ends, is wrong. AsyncIterator._end emits end synchronously and then calls removeAllListeners('end'), so a consumer that attaches its listeners after that point never sees the event. Ending as soon as the left stream ends is exactly such a point: nothing has necessarily read the join yet.

This is not hypothetical. It hangs Comunica's ActorRdfJoinHash test for empty input, which awaits metadata() before touching the stream. So the empty case sets a flag, and read ends the iterator, which mirrors how the non-empty case already ends from the right stream's end handler.

The added test ends for a listener that is attached after the left stream ended covers this and fails against the naive version.

Correctness

The short-circuit only fires when the hash table is empty after the left stream has ended, which for an inner join means the output is empty. _end already destroys both streams, so the right stream is cleaned up as before.

  • The existing suite passes unchanged, including merges an empty stream with a stream of length 3, can handle streams that are already done and propagates errors in the right stream.
  • Three tests are added: the right stream is never read, end still reaches a late listener, and a non-empty left stream whose join function filters everything out still reads the right stream and terminates.
  • Against Comunica with this linked in: the full Jest suite passes (6842 tests, 482 suites) and the SPARQL 1.0/1.1/1.2 specification suites pass.

CI

actions/cache@v2 is now refused outright by GitHub, so every run fails before installing anything, on master as well as here. The workflow is updated to actions/checkout@v4 and actions/setup-node@v4 with its built-in yarn cache, and the Node matrix moves from 12/14/16 to 20/22/24. Install uses --frozen-lockfile against the existing yarn.lock; no package-lock.json is introduced.

🤖 Generated with Claude Code

https://claude.ai/code/session_019fVTUvQ2uCR1p4KQxAYrgs

An inner hash join over an empty hash table produces nothing, so the right
stream never has to be read. It was read in full regardless, which is cheap for
an in-memory iterator but not for one backed by a remote source.

The iterator is ended from `read` rather than as soon as the left stream ends,
because `_end` notifies the listeners attached at that moment and then drops
them, and a consumer that is still being set up would never see the event.

In Comunica over a Triple Pattern Fragments endpoint this made three WatDiv
queries download every page of two fragments to produce no results:
S3 went from 7 to 67 HTTP requests, S5 from 7 to 28. Both are back to 7.

The CI workflow is updated alongside, since `actions/cache@v2` is now refused
by GitHub and the run fails before installing anything.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019fVTUvQ2uCR1p4KQxAYrgs
@rubensworks
rubensworks force-pushed the claude/hash-join-empty-build-shortcircuit branch from 0dc4046 to 38f0bb5 Compare September 7, 2026 13:23
@rubensworks
rubensworks merged commit d663a53 into master Sep 7, 2026
7 checks passed
@rubensworks
rubensworks deleted the claude/hash-join-empty-build-shortcircuit branch September 7, 2026 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants