[core] Fix integer overflow in FieldSumAgg causing silent data corruption#7338
Open
dubin555 wants to merge 1 commit intoapache:masterfrom
Open
[core] Fix integer overflow in FieldSumAgg causing silent data corruption#7338dubin555 wants to merge 1 commit intoapache:masterfrom
dubin555 wants to merge 1 commit intoapache:masterfrom
Conversation
…tion FieldSumAgg.agg() silently wraps on overflow for TINYINT, SMALLINT, INTEGER, and BIGINT types. For example, SUM(Byte.MAX_VALUE, 1) returns -128 instead of detecting the overflow. This causes silent data corruption in merge-tree aggregation when accumulator values exceed type bounds. Use Math.addExact/subtractExact/negateExact for INTEGER and BIGINT, and range-checked helpers for TINYINT and SMALLINT, to throw ArithmeticException on overflow instead of silently producing wrong results. Also fix the same issue in retract() and negative() methods.
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.
Purpose
FieldSumAggperforms unchecked arithmetic for TINYINT, SMALLINT, INTEGER, and BIGINT types. When the accumulator overflows, the result silently wraps around (e.g.,Byte.MAX_VALUE + 1becomes-128), producing incorrect aggregation results without any error or warning. This is silent data corruption in the merge-tree SUM aggregation path.The root cause differs by type:
intfor arithmetic, then the(byte)or(short)cast silently truncates the overflow bits.+and-wrap around atMAX_VALUE/MIN_VALUEper Java spec.This fix adds overflow detection to all three affected methods (
agg,retract,negative):Math.addExact/Math.subtractExact/Math.negateExact, which throwArithmeticExceptionon overflow.intwidth and verify the result fits inbyte/shortrange before casting.This is consistent with the overflow-protection pattern already used in
DecimalUtils.add()andDecimalUtils.subtract()in the same codebase.Tests
testFieldSumByteOverflow— TINYINT: verifies normal addition works, positive overflow throwsArithmeticException, negative overflow throws, and retract overflow throws.testFieldSumShortOverflow— SMALLINT: same coverage.testFieldSumIntOverflow— INTEGER: same coverage.testFieldSumLongOverflow— BIGINT: same coverage.API and Format
No API or storage format changes.
Documentation
No new feature introduced.
Generative AI tooling
Generated-by: Claude Code 1.0.0