This is an automated email from the ASF dual-hosted git repository.
pitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new f9eee02fd19 GH-54924: [C++] HasVariadicBuffers should respect
extension types (#54925)
f9eee02fd19 is described below
commit f9eee02fd19564c12ae58999fba6ac731be80836
Author: Alexander Taepper <[email protected]>
AuthorDate: Mon Oct 26 16:10:07 2026 +0100
GH-54924: [C++] HasVariadicBuffers should respect extension types (#54925)
### Rationale for this change
This fixes #54924. An extension array would not round-trip correctly
through ArraySpans as a test demonstrates.
### What changes are included in this PR?
This fixes the `HasVariadicBuffers` implementation to correctly handle
extension types. A test demonstrating the fix is also included.
The fix follows the implementation of `ArraySpan::SetMembers`, where the
same additional case for extension types is included
### Are these changes tested?
Yes. By the existing test suite plus an additional test.
### Are there any user-facing changes?
None other than the bug fix.
### Was AI used for this PR?
In accordance to the [AI generation
guidelines](https://arrow.apache.org/docs/dev/developers/overview.html#ai-generated-code),
please disclose below whether and how AI was used in this PR.
**PR code and description written by:**
- [X] Human
- [ ] AI
**Reviewed before submission by:**
- [X] Human
- [X] AI
- [ ] Not reviewed
An LLM suggested the second fix for
```
if (type_id == Type::STRING_VIEW || type_id == Type::BINARY_VIEW) {
```
in `ArraySpan::SetMembers`, which does look correct to me.
* GitHub Issue: #54924
Authored-by: Alexander Taepper <[email protected]>
Signed-off-by: Antoine Pitrou <[email protected]>
---
cpp/src/arrow/array/array_test.cc | 20 ++++++++++++++++++++
cpp/src/arrow/array/data.cc | 9 +++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/cpp/src/arrow/array/array_test.cc
b/cpp/src/arrow/array/array_test.cc
index 2cdf94bfab9..55ac106e690 100644
--- a/cpp/src/arrow/array/array_test.cc
+++ b/cpp/src/arrow/array/array_test.cc
@@ -1139,6 +1139,26 @@ TEST_F(TestArray, TestBinaryViewAppendArraySlice) {
AssertArraysEqual(*src, *dst);
}
+// GH-54924: ArraySpan::HasVariadicBuffers must look through extension types.
+TEST_F(TestArray, TestBinaryViewExtensionArraySpanRoundtrip) {
+ auto storage = ArrayFromJSON(
+ binary_view(),
+ R"([null, "short", "long string that is not inlined", "another somewhat
long string"])");
+ auto extension_type = binary_view_extension_type();
+ auto extension_array = ExtensionType::WrapArray(extension_type, storage);
+ ASSERT_OK(extension_array->ValidateFull());
+
+ ArraySpan span(*extension_array->data());
+ ASSERT_TRUE(span.HasVariadicBuffers());
+ ASSERT_EQ(span.GetVariadicBuffers().size(), storage->data()->buffers.size()
- 2);
+
+ auto roundtripped = span.ToArray();
+ ASSERT_OK(roundtripped->ValidateFull());
+ AssertArraysEqual(*extension_array, *roundtripped);
+ ASSERT_EQ(roundtripped->data()->buffers.size(),
+ extension_array->data()->buffers.size());
+}
+
TEST_F(TestArray, ValidateBuffersPrimitive) {
auto empty_buffer = std::make_shared<Buffer>("");
auto null_buffer = Buffer::FromString("\xff");
diff --git a/cpp/src/arrow/array/data.cc b/cpp/src/arrow/array/data.cc
index edeb1c31190..5e34758c0bc 100644
--- a/cpp/src/arrow/array/data.cc
+++ b/cpp/src/arrow/array/data.cc
@@ -321,7 +321,7 @@ void ArraySpan::SetMembers(const ArrayData& data) {
this->buffers[i] = {};
}
- if (type_id == Type::STRING_VIEW || type_id == Type::BINARY_VIEW) {
+ if (HasVariadicBuffers()) {
// store the span of data buffers in the third buffer
this->buffers[2] =
internal::PackVariadicBuffers(std::span(data.buffers).subspan(2));
}
@@ -687,7 +687,12 @@ std::span<const std::shared_ptr<Buffer>>
ArraySpan::GetVariadicBuffers() const {
}
bool ArraySpan::HasVariadicBuffers() const {
- return type->id() == Type::BINARY_VIEW || type->id() == Type::STRING_VIEW;
+ Type::type type_id = type->id();
+ if (type_id == Type::EXTENSION) {
+ const auto* ext_type = checked_cast<const ExtensionType*>(type);
+ type_id = ext_type->storage_type()->id();
+ }
+ return type_id == Type::BINARY_VIEW || type_id == Type::STRING_VIEW;
}
std::shared_ptr<Array> ArraySpan::ToArray() const {
