Repository navigation
Conversation
ScalarValue::to_array_of_size called StringArray::new_repeated, LargeStringArray::new_repeated, BinaryArray::new_repeated, and LargeBinaryArray::new_repeated directly, which panic when value.len() * size overflows the offset type (i32::MAX / i64::MAX) or usize. Extract a shared try_repeated_byte_array<T: ByteArrayType> helper that checks checked_mul, T::Offset::from_usize (returning ArrowError::OffsetOverflowError), and isize::try_from before constructing the array. Add unit tests in datafusion/common/src/scalar/mod.rs and an end-to-end SQL test in datafusion/sqllogictest/test_files/string/string_literal.slt for apache#27277. Signed-off-by: Ebel Thomas Seiko <ebelthomasseiko@gmail.com>
github-actions
Bot
added
sqllogictest
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.
Which issue does this PR close?
ScalarValue::to_array_of_sizepanics when aUtf8orBinaryscalar repeatedsizetimes passesi32::MAXbytes #27277.Rationale for this change
Calling
ScalarValue::to_array_of_sizeonUtf8,LargeUtf8,Binary, orLargeBinaryscalars panics whenvalue.len() * sizeoverflows the array offset type (i32::MAXforUtf8/Binary,i64::MAXforLargeUtf8/LargeBinary) orusize, becauseGenericByteArray::new_repeatedinvokesOffsetBuffer::from_repeated_length, which panics on overflow. Queries such asSELECT repeat('a', 1048576) AS s FROM generate_series(1, 2048)crash instead of returning an error.What changes are included in this PR?
try_repeated_byte_array<T: ByteArrayType>indatafusion/common/src/scalar/mod.rsand route theUtf8,LargeUtf8,Binary, andLargeBinaryarms inScalarValue::to_array_of_sizethrough it.usizemultiplication overflow,T::Offsetoverflow (returningArrowError::OffsetOverflowError), andisize::MAXaddress-space limits (forLarge*on 32-bit targets) before callingGenericByteArray::new_repeated.ScalarValue::to_array_of_size's# Errorssection.What is the testing strategy for this PR?
datafusion/common/src/scalar/mod.rs(test_to_array_of_size_byte_offset_overflow) coveringi32::MAXoverflow (Utf8,Binary),usizemultiplication overflow,i64::MAXoverflow (LargeUtf8,LargeBinaryon 64-bit), and valid empty/zero-size/non-overflow cases.sqllogictestquery indatafusion/sqllogictest/test_files/string/string_literal.sltexecuting the SQL reproduction (SELECT repeat('a', 1048576) AS s FROM generate_series(1, 2048);).main(9e0a08fd8): confirmed it panics atarrow-buffer'sOffsetBuffer::from_repeated_lengthwithoffset overflow: 2147483648 does not fit in i32, while with this patch it returnsDataFusion error: Arrow error: Offset overflow error: 2147483648.Are there any user-facing changes?
Queries that previously panicked on offset overflow in
ScalarValue::to_array_of_sizenow fail gracefully with anArrowError::OffsetOverflowErrororDataFusionError::Executionerror.