Copilot commented on code in PR #50929:
URL: https://github.com/apache/arrow/pull/50929#discussion_r3823513061
##########
cpp/src/arrow/array/array_nested.cc:
##########
@@ -1001,6 +1001,40 @@ Result<std::shared_ptr<Array>>
FixedSizeListArray::Flatten(
return FlattenListArray(*this, memory_pool);
}
+Result<std::shared_ptr<Tensor>> FixedSizeListArray::ToTensor() const {
+ const auto* data = this->data().get();
+ auto type = this->type();
+ int64_t offset = data->offset;
+ int64_t length = data->length;
+ std::vector<int64_t> shape{length};
+
+ // Iterate over nested fixed length container types.
+ // Each nested container increase the tensor dimension.
+ while (type->id() == Type::FIXED_SIZE_LIST) {
+ const auto* fsl = internal::checked_cast<const
FixedSizeListType*>(type.get());
+ type = fsl->value_type();
+ data = data->child_data.front().get();
+
+ offset = offset * fsl->list_size() + data->offset;
+ length *= fsl->list_size();
+ shape.push_back(fsl->list_size());
+ }
+
+ // Only checking byte_width and leaving Tensor::Make error on unsupported
types.
+ if (!is_fixed_width(*type)) {
+ return Status::NotImplemented("Expected a fixed width leaf type, got ",
type->name());
+ }
+
+ std::shared_ptr<Buffer> buffer = nullptr;
+ if (const auto& buf = data->buffers[1]; buf != NULLPTR) {
+ const int64_t byte_width = type->byte_width();
+ ARROW_ASSIGN_OR_RAISE(buffer,
+ SliceBufferSafe(buf, offset * byte_width, length *
byte_width));
+ }
Review Comment:
FixedSizeListArray::ToTensor computes `offset`, `length`, and `offset *
byte_width` / `length * byte_width` using unchecked int64 arithmetic. For large
offsets / deeply nested list sizes this can overflow before SliceBufferSafe
validates bounds, leading to incorrect slicing or undefined behavior. Consider
using Arrow's overflow-checked helpers (e.g.,
MultiplyWithOverflow/AddWithOverflow) when accumulating offsets and byte sizes
and returning Status::Invalid on overflow.
##########
cpp/src/arrow/array/array_primitive.h:
##########
@@ -128,6 +130,19 @@ class NumericArray : public PrimitiveArray {
IteratorType end() const { return IteratorType(*this, length()); }
+ /// \brief Return a one dimensional Tensor.
+ Result<std::shared_ptr<Tensor>> ToTensor() const override {
+ // Could be non-templated
+ const int64_t byte_width = type()->byte_width();
+ std::shared_ptr<Buffer> buffer;
+ if (data_->buffers[1] != NULLPTR) {
+ const auto boffset = data_->offset * byte_width;
+ const auto blength = length() * byte_width;
+ ARROW_ASSIGN_OR_RAISE(buffer, SliceBufferSafe(data_->buffers[1],
boffset, blength));
+ }
Review Comment:
NumericArray::ToTensor computes `data_->offset * byte_width` and `length() *
byte_width` without overflow checks. With very large arrays this can overflow
int64 before SliceBufferSafe runs, producing an invalid slice. Use
overflow-checked multiplication and return Status::Invalid if the byte
calculations overflow.
--
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]