ngoldbaum opened a new issue, #51156: URL: https://github.com/apache/arrow/issues/51156
### Describe the bug, including details regarding any error messages, version, and platform. `BaseBinaryBuilder::Append(std::string_view)` and `ChunkedBinaryBuilder::Append(std::string_view)` [narrow the value size to `int32_t`](https://github.com/apache/arrow/blob/ce6fb9f5e2a81c9a12c22e95cd62d29923fbafc3/cpp/src/arrow/array/builder_binary.h#L86-L88) [before calling](https://github.com/apache/arrow/blob/ce6fb9f5e2a81c9a12c22e95cd62d29923fbafc3/cpp/src/arrow/array/builder_binary.h#L951-L954) the `(const uint8_t*, int32_t)` [overload](https://github.com/apache/arrow/blob/ce6fb9f5e2a81c9a12c22e95cd62d29923fbafc3/cpp/src/arrow/array/builder_binary.h#L69-L80). For a value of 2 GiB or more the narrowed length is negative or wraps, and `BaseBinaryBuilder::Append` only copies data when the length is positive. The result is a non-null value holding the wrong bytes. `ValidateFull` also passes. `BinaryViewBuilder` [handles the same input correctly](https://github.com/apache/arrow/blob/ce6fb9f5e2a81c9a12c22e95cd62d29923fbafc3/cpp/src/arrow/array/builder_binary.h#L622-L624) with `CapacityError: BinaryView or StringView elements cannot reference strings larger than 2GB`, and `LargeBinaryBuilder` stores the full value. I would expect the two 32-bit-offset builders to return a `CapacityError` the way `BinaryViewBuilder` does. The callers in pyarrow guard against this on their own, so I did not find a way to trigger it from Python today. I'm not fixing this in the followup PR I'm opening to supersede #42018 with, because this is a problem with the Arrow C++ API that should be fixed separately. However, adding support for NumPy `StringDType` does make it possible to hit this from pure Python. Reproducer, run against a build of main (54ea6d5bab), macOS arm64: ```cpp #include <iostream> #include <string> #include <string_view> #include <arrow/api.h> template <typename Builder> void Check(const char* name, Builder& builder, std::string_view value) { auto status = builder.Append(value); std::cout << name << ": Append -> " << status.ToString(); if (!status.ok()) { std::cout << std::endl; return; } std::shared_ptr<arrow::Array> array; if constexpr (std::is_same_v<Builder, arrow::internal::ChunkedBinaryBuilder>) { arrow::ArrayVector chunks; if (auto s = builder.Finish(&chunks); !s.ok()) { std::cout << s.ToString() << std::endl; return; } if (auto s = arrow::Concatenate(chunks).Value(&array); !s.ok()) { std::cout << s.ToString() << std::endl; return; } } else { if (auto s = builder.Finish(&array); !s.ok()) { std::cout << s.ToString() << std::endl; return; } } auto scalar = std::static_pointer_cast<arrow::BaseBinaryScalar>(array->GetScalar(0).ValueOrDie()); std::cout << ", length " << array->length() << ", stored value size " << scalar->value->size() << ", ValidateFull -> " << array->ValidateFull().ToString() << std::endl; } int main() { const std::string value(static_cast<size_t>(1) << 31, 'x'); std::cout << "value size " << value.size() << std::endl; arrow::internal::ChunkedBinaryBuilder chunked(1 << 24); Check("ChunkedBinaryBuilder", chunked, value); arrow::BinaryBuilder binary; Check("BinaryBuilder", binary, value); arrow::BinaryViewBuilder view; Check("BinaryViewBuilder", view, value); arrow::LargeBinaryBuilder large; Check("LargeBinaryBuilder", large, value); return 0; } ``` ``` $ clang++ -std=c++20 -I $ARROW_HOME/include repro.cc -o repro -L $ARROW_HOME/lib -larrow -Wl,-rpath,$ARROW_HOME/lib $ ./repro value size 2147483648 ChunkedBinaryBuilder: Append -> OK, length 1, stored value size 0, ValidateFull -> OK BinaryBuilder: Append -> OK, length 1, stored value size 0, ValidateFull -> OK BinaryViewBuilder: Append -> Capacity error: BinaryView or StringView elements cannot reference strings larger than 2GB LargeBinaryBuilder: Append -> OK, length 1, stored value size 2147483648, ValidateFull -> OK ``` If I set the value size like this instead: ``` const std::string value((static_cast<size_t>(1) << 32) + 8, 'x'); ``` (e.g. 4 GB + 8 bytes), I get: ``` value size 4294967304 ChunkedBinaryBuilder: Append -> OK, length 1, stored value size 8, ValidateFull -> OK BinaryBuilder: Append -> OK, length 1, stored value size 8, ValidateFull -> OK BinaryViewBuilder: Append -> Capacity error: BinaryView or StringView elements cannot reference strings larger than 2GB LargeBinaryBuilder: Append -> OK, length 1, stored value size 4294967304, ValidateFull -> OK ``` ### Component(s) C++ -- 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]
