fix: guard empty fixed-width vectors in CBloomFilter to avoid index-out-of-range panic - #25864
Conversation
…ut-of-range panic Signed-off-by: Sai Asish Y <say.apm35@gmail.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
|
gouhongshen
left a comment
gouhongshen
left a comment
There was a problem hiding this comment.
Codex automated review
No concrete regressions found. The guards correctly make empty fixed-width vectors no-ops, matching the existing varlena behavior and issue contract. Local focused tests could not run because Go is unavailable; PR CI reported success.
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep review completed against f58f026. No merge-blocking findings.
The zero-length guard is placed before both fixedData[0] and results[0] in all three fixed-width vector paths. TestVector returns an empty result, while AddVector and TestAndAddVector are no-ops and invoke no callbacks, matching the existing varlena semantics. The non-empty path is unchanged apart from an O(1) length check, and the change introduces no new ownership, allocation-growth, concurrency, or wait-for edges.
Local verification passed:
- focused regression test
- full pkg/common/bloomfilter test suite
- full package with -race
- go vet
Non-blocking follow-up: the regression test could pass a counting callback and assert zero invocations instead of passing nil. Fixed const-null vectors with logical length greater than zero and empty backing data are a separate pre-existing API boundary and should be handled independently; this PR neither introduces nor claims to fix it.
Approved.
Merge Queue Status
This pull request spent 10 seconds in the queue, including 1 second running CI. Required conditions to merge
|
5 participants
What type of PR is this?
Which issue(s) this PR fixes:
issue #25618
What this PR does / why we need it:
The three fixed-width
CBloomFiltermethods (testAndAddFixedVector,testFixedVector,addFixedVector) take&fixedData[0]and&results[0]unconditionally, which panics withindex out of range [0] with length 0when the input vector is empty. Their varlena counterparts (testAndAddVarlenaVector,testVarlenaVector,addVarlenaVector) already return early onv.Length() == 0, so behavior on empty input was inconsistent by type.This adds the same zero-length early return to the fixed-width path so
TestVector/AddVector/TestAndAddVectorare no-ops on an empty vector instead of panicking, matching the varlena behavior. A regression test covers all three fixed-width APIs.