Repository navigation
GH-53519: [Python] Fix StringArray/LargeStringArray.from_buffers to accept int64 arguments - #53728
Closed
Asthenia0412 wants to merge 1 commit into
Closed
Asthenia0412 wants to merge 1 commit into
Asthenia0412 wants to merge 1 commit into
Conversation
…s to accept int64 arguments The StringArray.from_buffers() and LargeStringArray.from_buffers() methods declared their length, null_count and offset parameters as C int (32-bit), but the underlying C++ ArrayData counters are int64_t. This caused OverflowError when passing values above INT32_MAX. Widen all three parameters to int64_t in both methods so that the specialized factories accept the same valid arrays as the generic Array.from_buffers() does. Closes apache#53519
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.
Background
GH-53519 reports that
StringArray.from_buffers()andLargeStringArray.from_buffers()reject valid 64-bit array counters withOverflowError: value too large to convert to int. The issue is that these specialized factory methods declare theirlength,null_count, andoffsetparameters as Cythonint(32-bit C int), while the underlying ArrowArrayDatacounters areint64_t.In contrast, the generic
Array.from_buffers()method accepts unbounded Python integers and passes them through correctly to the C++ layer, which acceptsint64_tparameters inMakeWithChildren().Problem
When users pass valid values above
INT32_MAX(2^31) to the specializedStringArray.from_buffers()orLargeStringArray.from_buffers(), Cython throws anOverflowErrorbecause the Python integer cannot fit into a 32-bit Cint. This unnecessarily limits the API compared to the genericArray.from_buffers().Solution
Widen the three parameters (
length,null_count,offset) from Cythoninttoint64_tin bothStringArray.from_buffers()andLargeStringArray.from_buffers(). This aligns with:ArrayData::MakeWithChildren()signature which usesint64_tArray.from_buffers()which already accepts large integersDictionaryArray.from_buffers()already usesint64_t length)Testing
The change is a straightforward type widening:
int64_t, so no C++ changes are neededINT64_MAXwill now be accepted (though practical use is limited by address space)This fix was verified by code analysis against:\n- The issue's reproduction case\n- The
_ensure_compression()patterns\n- Cython declaration conventions used elsewhere in the same file\n\n\nCloses: #53519