Sriniketh24 commented on PR #50024: URL: https://github.com/apache/arrow/pull/50024#issuecomment-5531163621
Thanks for the pointer @AlenkaF — that discussion was exactly the missing piece. @bkietz caught a real bug in the approach I'd pushed: checking `length_` alone against `OffsetType::max()` is a false positive — an empty string repeated more than `int32::max()` times is perfectly valid (every offset stays 0), so that check would have incorrectly rejected it. Applied bkietz's suggested fix directly: compute `value_length * length_` in `int64_t` and compare the product against `OffsetType::max()`, rather than checking either operand individually. Also added the boundary tests js8544 asked for (empty string at length > int32::max, and "aa" at exactly int32::max/2) — these need multi-GB allocations to actually succeed rather than fail-fast, so I gated them behind `LARGE_MEMORY_TEST` like the rest of the codebase does (e.g. `table_test.cc`) rather than adding that cost to every CI run. Local syntax check (now with gmock available) is clean on both files. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
