fix: prevent division-by-zero and integer overflow in partition()#899
Open
hieuit095 wants to merge 1 commit into
Open
fix: prevent division-by-zero and integer overflow in partition()#899hieuit095 wants to merge 1 commit into
hieuit095 wants to merge 1 commit into
Conversation
- Guard num==0 to throw an error instead of panicking on division by zero - Guard inverted/empty range (max < min) to return a single query safely - Use checked_add/sub/mul to prevent i64 overflow in partition boundaries - Use saturating arithmetic to handle edge cases gracefully Fixes a crash when partition_query.num is set to 0 or when min/max values cause overflow in (max - min + 1) / num computation.
| let lower = match min.checked_add((i as i64).saturating_mul(partition_size)) { | ||
| Some(l) => l, | ||
| None => { | ||
| // Overflow computing lower bound; clamp to min. |
Contributor
There was a problem hiding this comment.
Won't this result in multiple partitions with overlapping ranges? This may cause the wrong result.
wangxiaoying
requested changes
Apr 17, 2026
Contributor
wangxiaoying
left a comment
There was a problem hiding this comment.
Thanks @hieuit095 for the PR!
I think the partition num check and max-min+1 overflow looks good. But the clamping logic may cause the same range handled by multiple partitions and result in the wrong result. We could just throw an error on those cases.
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
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.
Summary
Fixes a crash bug in
partition()(connectorx/src/partition.rs) where invalid inputs cause a panic via:num == 0in the calculation(max - min + 1) / nummin/maxvalues cause overflow in(max - min + 1)ori * partition_sizeBoth issues can be triggered by a caller passing a malicious or erroneous
partition_querywithnum == 0.Changes
num == 0: Returns a proper error instead of panicking on division by zeromax < min): Returns a single safe query rather than looping infinitelychecked_add/sub/mul: Prevents i64 overflow in partition boundary calculationssaturating_add: Safely handles edge cases wherepartition_sizewould be 0Impact
partition_sql()orread_sql()withpartition_query.num = 0would previously get an unhandled panicmin/maxvalues could cause silent wrong partition boundariesGenerated by git_pr bounty hunter agent.