Skip to content

fix(cudf): Guard hash join build debug logging against empty inputs#17221

Closed
pramodsatya wants to merge 1 commit intofacebookincubator:mainfrom
pramodsatya:fix-tpch-q16
Closed

fix(cudf): Guard hash join build debug logging against empty inputs#17221
pramodsatya wants to merge 1 commit intofacebookincubator:mainfrom
pramodsatya:fix-tpch-q16

Conversation

@pramodsatya
Copy link
Copy Markdown
Collaborator

@pramodsatya pramodsatya commented Apr 17, 2026

When the build side of a hash join has no data (e.g. anti-join with
empty result), inputs_ is empty. With a debug build, the logging at
noMoreInput() accesses inputs_[0] without bound checking,
which could result in a SIGSEGV.

This fixes Q16 (TPC-H) which uses NOT IN (anti-join) where the build
side can be empty for some partitions.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 17, 2026

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 05f80f0
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/69e18b451f3c9500085f1fd2

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 17, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 17, 2026

Build Impact Analysis

Full build recommended. Files outside the dependency graph changed:

  • velox/experimental/cudf/exec/CudfHashJoin.cpp
  • velox/experimental/cudf/tests/HashJoinTest.cpp

These directories are not fully covered by the dependency graph. A full build is the safest option.

cmake --build _build/release

Fast path • Graph from main@560a4edaf3efb2b0f0071f9eaf07c32919b58b50

@pramodsatya pramodsatya changed the title feat(cudf): Guard hash join build debug logging against empty inputs fix(cudf): Guard hash join build debug logging against empty inputs Apr 17, 2026
Copy link
Copy Markdown
Contributor

@jinchengchenghh jinchengchenghh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your fix.
Please update your PR description, this only occurred in debug mode

@pramodsatya
Copy link
Copy Markdown
Collaborator Author

Thanks @jinchengchenghh @devavret, updated the PR description. Could you help get this merged?

@devavret devavret added ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall cudf cudf related - GPU acceleration labels Apr 20, 2026
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Apr 20, 2026

@peterenescu has imported this pull request. If you are a Meta employee, you can view this in D101671299.

@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Apr 21, 2026

@peterenescu merged this pull request in 0187d5b.

shrshi pushed a commit to patdevinwilson/velox that referenced this pull request Apr 23, 2026
…acebookincubator#17221)

Summary:
When the build side of a hash join has no data (e.g. anti-join with
empty result), inputs_ is empty. With a debug build, the logging at
`noMoreInput()` accesses `inputs_[0]` without bound checking,
which could result in a `SIGSEGV`.

This fixes Q16 (TPC-H) which uses NOT IN (anti-join) where the build
side can be empty for some partitions.

Pull Request resolved: facebookincubator#17221

Reviewed By: kagamiori

Differential Revision: D101671299

Pulled By: peterenescu

fbshipit-source-id: 0971d85671e67810789d8148ce3891e757faf4f7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. cudf cudf related - GPU acceleration Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants