feat(cpp): Implement ViewBinaryViewArrayAsBytes (#633)

apache · Jan 29, 2025 · bbcea42 · bbcea42
1 parent 4ac2c1c
commit bbcea42
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 27 deletions.
diff --git a/src/nanoarrow/hpp/view.hpp b/src/nanoarrow/hpp/view.hpp
@@ -243,16 +243,16 @@ class ViewBinaryViewArrayAsBytes {
 
     internal::Maybe<ArrowStringView> operator()(int64_t i) const {
       if (validity == nullptr || ArrowBitGet(validity, i)) {
-        const union ArrowBinaryView bv = inline_data[i];
-        if (bv.inlined.size <= NANOARROW_BINARY_VIEW_INLINE_SIZE) {
-          return ArrowStringView{
-              reinterpret_cast<const char*>(inline_data[i].inlined.data),
-              bv.inlined.size};
+        const union ArrowBinaryView* bv = &inline_data[i];
+        if (bv->inlined.size <= NANOARROW_BINARY_VIEW_INLINE_SIZE) {
+          return ArrowStringView{reinterpret_cast<const char*>(bv->inlined.data),
+                                 bv->inlined.size};
         }
 
         return ArrowStringView{
-            reinterpret_cast<const char*>(variadic_buffers[bv.ref.buffer_index]),
-            bv.inlined.size};
+            reinterpret_cast<const char*>(variadic_buffers[bv->ref.buffer_index]) +
+                bv->ref.offset,
+            bv->ref.size};
       }
       return NA;
     }
@@ -272,18 +272,14 @@ class ViewBinaryViewArrayAsBytes {
             array_view->length,
         } {}
 
-  /*
   ViewBinaryViewArrayAsBytes(const ArrowArray* array)
       : range_{
-            Get{
-                static_cast<const uint8_t*>(array->buffers[0]),
-                array->buffers[1],
-                static_cast<const char*>(array->buffers[2]),
-            },
+            Get{static_cast<const uint8_t*>(array->buffers[0]),
+                static_cast<const union ArrowBinaryView*>(array->buffers[1]),
+                array->buffers + NANOARROW_BINARY_VIEW_FIXED_BUFFERS},
             array->offset,
             array->length,
         } {}
-  */
 
   using value_type = typename internal::RandomAccessRange<Get>::value_type;
   using const_iterator = typename internal::RandomAccessRange<Get>::const_iterator;

diff --git a/src/nanoarrow/hpp/view_test.cc b/src/nanoarrow/hpp/view_test.cc
@@ -82,28 +82,44 @@ TEST(NanoarrowHppTest, NanoarrowHppViewArrayAsBytesTest) {
   }
 }
 
-TEST(NanoarrowHppTest, NanoarrowHppViewBinaryViewArrayAsBytesTest) {
+class BinaryViewTypeTestFixture : public ::testing::TestWithParam<enum ArrowType> {
+ protected:
+  enum ArrowType data_type;
+};
+
+TEST_P(BinaryViewTypeTestFixture, NanoarrowHppViewBinaryViewArrayAsBytesTest) {
   using namespace nanoarrow::literals;
 
   nanoarrow::UniqueArray array{};
-  NANOARROW_THROW_NOT_OK(ArrowArrayInitFromType(array.get(), NANOARROW_TYPE_BINARY_VIEW));
-  NANOARROW_THROW_NOT_OK(ArrowArrayStartAppending(array.get()));
+  ASSERT_EQ(ArrowArrayInitFromType(array.get(), GetParam()), NANOARROW_OK);
+  ASSERT_EQ(ArrowArrayStartAppending(array.get()), NANOARROW_OK);
 
-  NANOARROW_THROW_NOT_OK(ArrowArrayAppendString(array.get(), "foo"_asv));
-  NANOARROW_THROW_NOT_OK(ArrowArrayAppendNull(array.get(), 1));
-  NANOARROW_THROW_NOT_OK(
-      ArrowArrayAppendString(array.get(), "this_string_is_longer_than_inline"_asv));
-  NANOARROW_THROW_NOT_OK(ArrowArrayAppendNull(array.get(), 1));
-  NANOARROW_THROW_NOT_OK(ArrowArrayFinishBuildingDefault(array.get(), nullptr));
+  ASSERT_EQ(ArrowArrayAppendString(array.get(), "foo"_asv), NANOARROW_OK);
+  ASSERT_EQ(ArrowArrayAppendNull(array.get(), 1), NANOARROW_OK);
+  ASSERT_EQ(ArrowArrayAppendString(array.get(), "this_string_is_longer_than_inline"_asv),
+            NANOARROW_OK);
+  ASSERT_EQ(ArrowArrayAppendNull(array.get(), 1), NANOARROW_OK);
+  ASSERT_EQ(ArrowArrayFinishBuildingDefault(array.get(), nullptr), NANOARROW_OK);
 
-  int i = 0;
   ArrowStringView expected[] = {"foo"_asv, ""_asv,
-                                "this_string_is_longer_than_inline"_asv, ""_asv};
+                                "this_string_is_longer_than_inline"_asv, ""_asv,
+                                "here_is_another_string"_asv};
 
   nanoarrow::UniqueArrayView array_view{};
-  ArrowArrayViewInitFromType(array_view.get(), NANOARROW_TYPE_BINARY_VIEW);
-  NANOARROW_THROW_NOT_OK(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr));
+  ArrowArrayViewInitFromType(array_view.get(), GetParam());
+  ASSERT_EQ(ArrowArrayViewSetArray(array_view.get(), array.get(), nullptr), NANOARROW_OK);
 
+  int i = 0;
+  for (auto slot : nanoarrow::ViewBinaryViewArrayAsBytes(array.get())) {
+    if (i == 1 || i == 3) {
+      EXPECT_EQ(slot, nanoarrow::NA);
+    } else {
+      EXPECT_EQ(slot, expected[i]);
+    }
+    ++i;
+  }
+
+  i = 0;
   for (auto slot : nanoarrow::ViewBinaryViewArrayAsBytes(array_view.get())) {
     if (i == 1 || i == 3) {
       EXPECT_EQ(slot, nanoarrow::NA);
@@ -114,6 +130,10 @@ TEST(NanoarrowHppTest, NanoarrowHppViewBinaryViewArrayAsBytesTest) {
   }
 }
 
+INSTANTIATE_TEST_SUITE_P(NanoarrowHppTest, BinaryViewTypeTestFixture,
+                         ::testing::Values(NANOARROW_TYPE_BINARY_VIEW,
+                                           NANOARROW_TYPE_STRING_VIEW));
+
 TEST(NanoarrowHppTest, NanoarrowHppViewArrayAsFixedSizeBytesTest) {
   using namespace nanoarrow::literals;