[GLUTEN-12504][VL] Fix two null dereferences on BHJ stages in the micro benchmark - #12592
Open
malinjawi wants to merge 1 commit into
Open
[GLUTEN-12504][VL] Fix two null dereferences on BHJ stages in the micro benchmark#12592malinjawi wants to merge 1 commit into
malinjawi wants to merge 1 commit into
Conversation
malinjawi
changed the title
malinjawi
force-pushed
the
fix/gluten-12504-bhj-microbenchmark
branch
from
August 5, 2026 08:57
c8b995d to
0dc95cd
Compare
malinjawi
changed the title
malinjawi
force-pushed
the
fix/gluten-12504-bhj-microbenchmark
branch
from
August 10, 2026 09:23
0dc95cd to
e160790
Compare
…ro benchmark getJoin() dereferenced JniHashTableContext::vm_, which is only set from JNI_OnLoad, so converting a plan with a non-empty hashTableId crashed in a process with no JVM. callJavaGet() now returns std::optional and getJoin() reports a miss, the same way the JVM side cache reports one. VeloxColumnarBatchWriter creates the parquet writer on the first batch, where it learns the schema, so an input iterator that yields nothing left it null and close() dereferenced it. A BHJ build side is such an iterator. close() is now a no-op when nothing was written, and the dumper records the gap as a .empty marker rather than reading back a file that was never created.
malinjawi
force-pushed
the
fix/gluten-12504-bhj-microbenchmark
branch
from
August 10, 2026 10:01
e160790 to
be3857f
Compare
malinjawi
marked this pull request as ready for review
August 10, 2026 10:04
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
Part of #12504.
A stage containing a broadcast hash join could not be dumped or replayed, because of a null dereference on each side.
Replay:
getJoin()dereferencedJniHashTableContext::vm_, which is only set fromJNI_OnLoad, so converting a plan with a non-emptyhashTableIdcrashed in a process with no JVM. Regression from #12487.callJavaGet()now returnsstd::optionalandgetJoin()reports a miss, the same0the JVM side cache returns and the converter already falls back on.Dump:
VeloxColumnarBatchWritercreates the parquet writer on the first batch, where it learns the schema, so an input iterator that yields nothing left it null andclose()dereferenced it. A BHJ build side is such an iterator.close()is now a no-op when nothing was written, and the dumper records the gap as a.emptymarker instead of reading back a file that was never created.This does not make the benchmark exercise the real BHJ path. That is #12738.
How was this patch tested?
New
jni_hash_table_testandvelox_whole_stage_dumper_test, both running in the no-JVM configuration the benchmark uses.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code