brijrajk opened a new pull request, #12614:
URL: https://github.com/apache/gluten/pull/12614
## What changes are proposed in this pull request?
Fixes #12613.
`VeloxBloomFilterAggregate` (the JVM-side expression used for Velox bloom
filters) and the
native `bloom_filter_agg` Velox function size their internal bit array
differently, even when
given the identical `estimatedNumItems`/`numBits` arguments:
- JVM `VeloxBloomFilterAggregate.createAggregationBuffer()` called
`VeloxBloomFilter.empty(estimatedNumItems)`, sizing the buffer purely from
the raw item
count and ignoring `numBits` entirely.
- Native `BloomFilterAggAggregate.cpp` computes `capacity_ = min(numBits,
maxNumBits) / 16`
and ignores the raw item count once `numBits` is known.
For example, given `estimatedNumItems=1000000, numBits=8388608` (Spark's
plain defaults),
the JVM side allocated a 16,777,216-bit buffer while the native side
allocated an
8,388,608-bit buffer -- exactly 2x different, deterministically, for any
query using this
code path.
### Why this matters
Two-phase aggregation runs the partial and final stages as separate physical
operators,
which can independently land on the JVM or on native Velox (e.g. via
Gluten's whole-stage
fallback policy, or via `spark.gluten.sql.columnar.hashagg.enabled=false`).
When the partial
aggregate runs on one engine and the final aggregate (which merges partial
buffers) runs on
the other, the merge combines two differently-sized bit arrays.
Velox's `BloomFilter::merge` (`velox/common/base/BloomFilter.h`) guards the
size match with
`VELOX_DCHECK_EQ`, which is compiled out in release builds, so the
mismatched merge proceeds
silently instead of throwing. The result is silent data corruption: bits
inserted relative
to one array size are later queried relative to a different array size,
causing bloom
filter false negatives (values that were definitely inserted are reported as
absent).
### The fix
Make `VeloxBloomFilterAggregate`'s JVM-side buffer sizing use the same
formula as the native
side (derive capacity from `numBits`, not from raw `estimatedNumItems`), so
both engines
agree on capacity for the same input arguments regardless of which engine
executes which
stage.
## How was this patch tested?
Added a regression test to `GlutenBloomFilterAggregateQuerySuite` that runs
the same
`bloom_filter_agg` query fully natively and fully on the JVM
(`spark.gluten.sql.columnar.hashagg.enabled=false`), and asserts the two
produce
identically-sized serialized bytes.
Verified the test actually catches the bug: reverting only the fix (keeping
the new test)
causes exactly this one test to fail, with 14 other tests in the same suite
unaffected.
With the fix, all 15 tests in `GlutenBloomFilterAggregateQuerySuite` pass.
## Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Sonnet 5 (Anthropic)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
