paleolimbot commented on code in PR #930:
URL: https://github.com/apache/arrow-nanoarrow/pull/930#discussion_r3918544331


##########
src/nanoarrow/common/array.c:
##########
@@ -77,6 +77,174 @@ int ArrowArrayIsInternal(struct ArrowArray* array) {
   return array->release == &ArrowArrayReleaseInternal;
 }
 
+static ArrowErrorCode ArrowArrayAppendArrayViewElement(struct ArrowArray* dst,
+                                                       const struct 
ArrowArrayView* src,
+                                                       int64_t i,
+                                                       struct ArrowError* 
error) {
+  if (ArrowArrayViewIsNull(src, i)) {
+    return ArrowArrayAppendNull(dst, 1);
+  }
+
+  switch (src->storage_type) {
+    case NANOARROW_TYPE_NA:
+      return ArrowArrayAppendNull(dst, 1);
+    case NANOARROW_TYPE_BOOL:
+    case NANOARROW_TYPE_INT8:
+    case NANOARROW_TYPE_INT16:
+    case NANOARROW_TYPE_INT32:
+    case NANOARROW_TYPE_INT64:
+    case NANOARROW_TYPE_DATE32:
+    case NANOARROW_TYPE_DATE64:
+    case NANOARROW_TYPE_TIMESTAMP:
+    case NANOARROW_TYPE_TIME32:
+    case NANOARROW_TYPE_TIME64:
+    case NANOARROW_TYPE_DURATION:
+      return ArrowArrayAppendInt(dst, ArrowArrayViewGetIntUnsafe(src, i));
+    case NANOARROW_TYPE_UINT8:
+    case NANOARROW_TYPE_UINT16:
+    case NANOARROW_TYPE_UINT32:
+    case NANOARROW_TYPE_UINT64:
+      return ArrowArrayAppendUInt(dst, ArrowArrayViewGetUIntUnsafe(src, i));
+    case NANOARROW_TYPE_HALF_FLOAT:
+    case NANOARROW_TYPE_FLOAT:
+    case NANOARROW_TYPE_DOUBLE:
+      return ArrowArrayAppendDouble(dst, ArrowArrayViewGetDoubleUnsafe(src, 
i));
+    case NANOARROW_TYPE_STRING:
+    case NANOARROW_TYPE_BINARY:
+    case NANOARROW_TYPE_FIXED_SIZE_BINARY:
+    case NANOARROW_TYPE_LARGE_STRING:
+    case NANOARROW_TYPE_LARGE_BINARY:
+    case NANOARROW_TYPE_BINARY_VIEW:
+    case NANOARROW_TYPE_STRING_VIEW:
+      return ArrowArrayAppendBytes(dst, ArrowArrayViewGetBytesUnsafe(src, i));
+    case NANOARROW_TYPE_INTERVAL_MONTHS:
+    case NANOARROW_TYPE_INTERVAL_DAY_TIME:
+    case NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO: {
+      struct ArrowInterval interval;
+      ArrowIntervalInit(&interval, src->storage_type);
+      ArrowArrayViewGetIntervalUnsafe(src, i, &interval);
+      return ArrowArrayAppendInterval(dst, &interval);
+    }
+    case NANOARROW_TYPE_DECIMAL32:
+    case NANOARROW_TYPE_DECIMAL64:
+    case NANOARROW_TYPE_DECIMAL128:
+    case NANOARROW_TYPE_DECIMAL256: {
+      int32_t bitwidth = 32;
+      if (src->storage_type == NANOARROW_TYPE_DECIMAL64) {
+        bitwidth = 64;
+      } else if (src->storage_type == NANOARROW_TYPE_DECIMAL128) {
+        bitwidth = 128;
+      } else if (src->storage_type == NANOARROW_TYPE_DECIMAL256) {
+        bitwidth = 256;
+      }
+
+      struct ArrowDecimal decimal;
+      ArrowDecimalInit(&decimal, bitwidth, /*precision=*/0, /*scale=*/0);
+      ArrowArrayViewGetDecimalUnsafe(src, i, &decimal);
+      return ArrowArrayAppendDecimal(dst, &decimal);
+    }
+    case NANOARROW_TYPE_STRUCT:
+      for (int64_t child_i = 0; child_i < src->n_children; child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[child_i], src->children[child_i], src->offset + i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    case NANOARROW_TYPE_LIST:
+    case NANOARROW_TYPE_LARGE_LIST:
+    case NANOARROW_TYPE_MAP:
+    case NANOARROW_TYPE_LIST_VIEW:
+    case NANOARROW_TYPE_LARGE_LIST_VIEW: {
+      int64_t logical_i = src->offset + i;
+      int64_t child_offset = ArrowArrayViewListChildOffset(src, logical_i);
+      int64_t child_length;
+      if (src->storage_type == NANOARROW_TYPE_LIST_VIEW) {
+        child_length = src->buffer_views[2].data.as_int32[logical_i];
+      } else if (src->storage_type == NANOARROW_TYPE_LARGE_LIST_VIEW) {
+        child_length = src->buffer_views[2].data.as_int64[logical_i];
+      } else {
+        child_length = ArrowArrayViewListChildOffset(src, logical_i + 1) - 
child_offset;
+      }
+
+      for (int64_t child_i = 0; child_i < child_length; child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[0], src->children[0], child_offset + child_i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    }
+    case NANOARROW_TYPE_FIXED_SIZE_LIST: {
+      int64_t child_offset = (src->offset + i) * 
src->layout.child_size_elements;
+      for (int64_t child_i = 0; child_i < src->layout.child_size_elements; 
child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[0], src->children[0], child_offset + child_i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    }
+    case NANOARROW_TYPE_DENSE_UNION:
+    case NANOARROW_TYPE_SPARSE_UNION: {
+      int8_t type_id = ArrowArrayViewUnionTypeId(src, i);
+      int8_t child_index = ArrowArrayViewUnionChildIndex(src, i);
+      int64_t child_offset = ArrowArrayViewUnionChildOffset(src, i);
+      NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+          dst->children[child_index], src->children[child_index], 
child_offset, error));
+      NANOARROW_RETURN_NOT_OK(
+          ArrowBufferAppend(ArrowArrayBuffer(dst, 0), &type_id, 
sizeof(type_id)));
+      if (src->storage_type == NANOARROW_TYPE_DENSE_UNION) {
+        _NANOARROW_CHECK_RANGE(dst->children[child_index]->length - 1, 0, 
INT32_MAX);
+        NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt32(
+            ArrowArrayBuffer(dst, 1), 
(int32_t)dst->children[child_index]->length - 1));
+      } else {
+        for (int64_t child_i = 0; child_i < dst->n_children; child_i++) {
+          if (child_i != child_index && dst->children[child_i]->length == 
dst->length) {
+            
NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(dst->children[child_i], 1));
+          }
+          if (dst->children[child_i]->length != dst->length + 1) {
+            ArrowErrorSet(error,
+                          "Expected sparse union child length of %" PRId64
+                          " but found %" PRId64,
+                          dst->length + 1, dst->children[child_i]->length);
+            return EINVAL;
+          }
+        }
+      }
+      dst->length++;
+      return NANOARROW_OK;
+    }
+    case NANOARROW_TYPE_RUN_END_ENCODED:
+    case NANOARROW_TYPE_UNINITIALIZED:
+    default:
+      ArrowErrorSet(error, "Appending array views is not supported for %s",
+                    ArrowTypeString(src->storage_type));
+      return ENOTSUP;
+  }
+}
+
+ArrowErrorCode ArrowArrayAppendArrayView(struct ArrowArray* dst,
+                                         const struct ArrowArrayView* src,
+                                         struct ArrowError* error) {
+  if (src->storage_type == NANOARROW_TYPE_RUN_END_ENCODED) {

Review Comment:
   At the top of this function we should sanity check that the append makes 
sense, because I am not sure that all combinations of spurious comparisons will 
generate a reasonable error here. ArrowArrayView only gives you storage type, 
so we probably should name the function accordingly 
(`ArrowArrayAppendStorageFromArrayView()`) to make it a bit more obvious to the 
caller that they're on their own with respect to whether they *should* do this 
or not.
   
   Also a note that this needs to check `src->dictionary`, which I don't 
believe is reflected in the storage type (for better or worse).



##########
src/nanoarrow/common/array.c:
##########
@@ -77,6 +77,174 @@ int ArrowArrayIsInternal(struct ArrowArray* array) {
   return array->release == &ArrowArrayReleaseInternal;
 }
 
+static ArrowErrorCode ArrowArrayAppendArrayViewElement(struct ArrowArray* dst,
+                                                       const struct 
ArrowArrayView* src,
+                                                       int64_t i,
+                                                       struct ArrowError* 
error) {

Review Comment:
   While we are here, I think it is worth the effort to special case primitive 
fixed-size appends of the same storage type (e.g., int32 getting appended to an 
int32). These should be much faster because they are just appending buffers to 
each other (the nulls bitmap is a bit more complex...may have to go through a 
uint8 buffer for the bitmap append if either the source or the target has any 
nulls).



##########
src/nanoarrow/common/array_test.cc:
##########
@@ -5087,3 +5087,232 @@ TEST(ArrayMoveSharedTest, ArrayWithNullBuffers) {
 
   ArrowArrayRelease(&shared);
 }
+
+static ArrowErrorCode AppendArrayViewForTest(const struct ArrowArrayView* src,
+                                             struct ArrowArray* dst,
+                                             struct ArrowError* error) {
+  NANOARROW_RETURN_NOT_OK(ArrowArrayInitFromArrayView(dst, src, error));
+  NANOARROW_RETURN_NOT_OK(ArrowArrayStartAppending(dst));
+  NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayView(dst, src, error));
+  return ArrowArrayFinishBuildingDefault(dst, error);
+}
+
+static void ExpectPrimitiveArrayViewAppendIdentical(struct ArrowArray* src,
+                                                    enum ArrowType type) {
+  struct ArrowError error;
+  struct ArrowArrayView src_view;
+  ArrowArrayViewInitFromType(&src_view, type);
+  ASSERT_EQ(ArrowArrayViewSetArray(&src_view, src, &error), NANOARROW_OK)
+      << error.message;
+
+  struct ArrowArray dst;
+  ASSERT_EQ(AppendArrayViewForTest(&src_view, &dst, &error), NANOARROW_OK)
+      << error.message;
+  struct ArrowArrayView dst_view;
+  ArrowArrayViewInitFromType(&dst_view, type);
+  ASSERT_EQ(ArrowArrayViewSetArray(&dst_view, &dst, &error), NANOARROW_OK)
+      << error.message;
+
+  int identical = 0;
+  ASSERT_EQ(ArrowArrayViewCompare(&src_view, &dst_view, 
NANOARROW_COMPARE_IDENTICAL,
+                                  &identical, &error),
+            NANOARROW_OK);
+  EXPECT_EQ(identical, 1) << error.message;
+
+  ArrowArrayViewReset(&dst_view);
+  ArrowArrayRelease(&dst);
+  ArrowArrayViewReset(&src_view);
+}
+
+TEST(ArrayTest, ArrayAppendArrayViewPrimitiveTypes) {
+  struct ArrowArray array;
+
+  ASSERT_EQ(ArrowArrayInitFromType(&array, NANOARROW_TYPE_INT64), 
NANOARROW_OK);

Review Comment:
   Some things that I am not sure are covered here are:
   
   - Appending an integer that is out of range to a narrower integer type 
(should pass on the error from the integer appender)
   - Appending too many elements to a narrower list type (you can use a list of 
Null type to avoid allocating huge buffers), which should pass on an error from 
the list element finisher
   - Dictionary encoded source or target



##########
src/nanoarrow/common/array.c:
##########
@@ -77,6 +77,174 @@ int ArrowArrayIsInternal(struct ArrowArray* array) {
   return array->release == &ArrowArrayReleaseInternal;
 }
 
+static ArrowErrorCode ArrowArrayAppendArrayViewElement(struct ArrowArray* dst,
+                                                       const struct 
ArrowArrayView* src,
+                                                       int64_t i,
+                                                       struct ArrowError* 
error) {
+  if (ArrowArrayViewIsNull(src, i)) {
+    return ArrowArrayAppendNull(dst, 1);
+  }
+
+  switch (src->storage_type) {
+    case NANOARROW_TYPE_NA:
+      return ArrowArrayAppendNull(dst, 1);
+    case NANOARROW_TYPE_BOOL:
+    case NANOARROW_TYPE_INT8:
+    case NANOARROW_TYPE_INT16:
+    case NANOARROW_TYPE_INT32:
+    case NANOARROW_TYPE_INT64:
+    case NANOARROW_TYPE_DATE32:
+    case NANOARROW_TYPE_DATE64:
+    case NANOARROW_TYPE_TIMESTAMP:
+    case NANOARROW_TYPE_TIME32:
+    case NANOARROW_TYPE_TIME64:
+    case NANOARROW_TYPE_DURATION:
+      return ArrowArrayAppendInt(dst, ArrowArrayViewGetIntUnsafe(src, i));
+    case NANOARROW_TYPE_UINT8:
+    case NANOARROW_TYPE_UINT16:
+    case NANOARROW_TYPE_UINT32:
+    case NANOARROW_TYPE_UINT64:
+      return ArrowArrayAppendUInt(dst, ArrowArrayViewGetUIntUnsafe(src, i));
+    case NANOARROW_TYPE_HALF_FLOAT:
+    case NANOARROW_TYPE_FLOAT:
+    case NANOARROW_TYPE_DOUBLE:
+      return ArrowArrayAppendDouble(dst, ArrowArrayViewGetDoubleUnsafe(src, 
i));
+    case NANOARROW_TYPE_STRING:
+    case NANOARROW_TYPE_BINARY:
+    case NANOARROW_TYPE_FIXED_SIZE_BINARY:
+    case NANOARROW_TYPE_LARGE_STRING:
+    case NANOARROW_TYPE_LARGE_BINARY:
+    case NANOARROW_TYPE_BINARY_VIEW:
+    case NANOARROW_TYPE_STRING_VIEW:
+      return ArrowArrayAppendBytes(dst, ArrowArrayViewGetBytesUnsafe(src, i));
+    case NANOARROW_TYPE_INTERVAL_MONTHS:
+    case NANOARROW_TYPE_INTERVAL_DAY_TIME:
+    case NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO: {
+      struct ArrowInterval interval;
+      ArrowIntervalInit(&interval, src->storage_type);
+      ArrowArrayViewGetIntervalUnsafe(src, i, &interval);
+      return ArrowArrayAppendInterval(dst, &interval);
+    }
+    case NANOARROW_TYPE_DECIMAL32:
+    case NANOARROW_TYPE_DECIMAL64:
+    case NANOARROW_TYPE_DECIMAL128:
+    case NANOARROW_TYPE_DECIMAL256: {
+      int32_t bitwidth = 32;
+      if (src->storage_type == NANOARROW_TYPE_DECIMAL64) {
+        bitwidth = 64;
+      } else if (src->storage_type == NANOARROW_TYPE_DECIMAL128) {
+        bitwidth = 128;
+      } else if (src->storage_type == NANOARROW_TYPE_DECIMAL256) {
+        bitwidth = 256;
+      }
+
+      struct ArrowDecimal decimal;
+      ArrowDecimalInit(&decimal, bitwidth, /*precision=*/0, /*scale=*/0);
+      ArrowArrayViewGetDecimalUnsafe(src, i, &decimal);
+      return ArrowArrayAppendDecimal(dst, &decimal);
+    }
+    case NANOARROW_TYPE_STRUCT:
+      for (int64_t child_i = 0; child_i < src->n_children; child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[child_i], src->children[child_i], src->offset + i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    case NANOARROW_TYPE_LIST:
+    case NANOARROW_TYPE_LARGE_LIST:
+    case NANOARROW_TYPE_MAP:
+    case NANOARROW_TYPE_LIST_VIEW:
+    case NANOARROW_TYPE_LARGE_LIST_VIEW: {
+      int64_t logical_i = src->offset + i;
+      int64_t child_offset = ArrowArrayViewListChildOffset(src, logical_i);
+      int64_t child_length;
+      if (src->storage_type == NANOARROW_TYPE_LIST_VIEW) {
+        child_length = src->buffer_views[2].data.as_int32[logical_i];
+      } else if (src->storage_type == NANOARROW_TYPE_LARGE_LIST_VIEW) {
+        child_length = src->buffer_views[2].data.as_int64[logical_i];
+      } else {
+        child_length = ArrowArrayViewListChildOffset(src, logical_i + 1) - 
child_offset;
+      }
+
+      for (int64_t child_i = 0; child_i < child_length; child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[0], src->children[0], child_offset + child_i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    }
+    case NANOARROW_TYPE_FIXED_SIZE_LIST: {
+      int64_t child_offset = (src->offset + i) * 
src->layout.child_size_elements;
+      for (int64_t child_i = 0; child_i < src->layout.child_size_elements; 
child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[0], src->children[0], child_offset + child_i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    }
+    case NANOARROW_TYPE_DENSE_UNION:
+    case NANOARROW_TYPE_SPARSE_UNION: {
+      int8_t type_id = ArrowArrayViewUnionTypeId(src, i);
+      int8_t child_index = ArrowArrayViewUnionChildIndex(src, i);
+      int64_t child_offset = ArrowArrayViewUnionChildOffset(src, i);
+      NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+          dst->children[child_index], src->children[child_index], 
child_offset, error));
+      NANOARROW_RETURN_NOT_OK(
+          ArrowBufferAppend(ArrowArrayBuffer(dst, 0), &type_id, 
sizeof(type_id)));
+      if (src->storage_type == NANOARROW_TYPE_DENSE_UNION) {
+        _NANOARROW_CHECK_RANGE(dst->children[child_index]->length - 1, 0, 
INT32_MAX);
+        NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt32(
+            ArrowArrayBuffer(dst, 1), 
(int32_t)dst->children[child_index]->length - 1));
+      } else {
+        for (int64_t child_i = 0; child_i < dst->n_children; child_i++) {
+          if (child_i != child_index && dst->children[child_i]->length == 
dst->length) {
+            
NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(dst->children[child_i], 1));
+          }
+          if (dst->children[child_i]->length != dst->length + 1) {
+            ArrowErrorSet(error,
+                          "Expected sparse union child length of %" PRId64
+                          " but found %" PRId64,
+                          dst->length + 1, dst->children[child_i]->length);
+            return EINVAL;
+          }
+        }
+      }
+      dst->length++;
+      return NANOARROW_OK;
+    }
+    case NANOARROW_TYPE_RUN_END_ENCODED:
+    case NANOARROW_TYPE_UNINITIALIZED:
+    default:
+      ArrowErrorSet(error, "Appending array views is not supported for %s",
+                    ArrowTypeString(src->storage_type));
+      return ENOTSUP;
+  }
+}
+
+ArrowErrorCode ArrowArrayAppendArrayView(struct ArrowArray* dst,
+                                         const struct ArrowArrayView* src,
+                                         struct ArrowError* error) {
+  if (src->storage_type == NANOARROW_TYPE_RUN_END_ENCODED) {
+    if (src->offset != 0) {
+      ArrowErrorSet(error, "Can't append a sliced run-end encoded array view");
+      return ENOTSUP;
+    }

Review Comment:
   No need to get too into the weeds supporting this one, but we do have a 
utility that I think can resolve the first run:
   
   
https://github.com/apache/arrow-nanoarrow/blob/f55f85be3a1008bee99f556eb3cda3949356bbd8/src/nanoarrow/common/inline_buffer.h#L53-L54



##########
src/nanoarrow/common/array.c:
##########
@@ -77,6 +77,174 @@ int ArrowArrayIsInternal(struct ArrowArray* array) {
   return array->release == &ArrowArrayReleaseInternal;
 }
 
+static ArrowErrorCode ArrowArrayAppendArrayViewElement(struct ArrowArray* dst,
+                                                       const struct 
ArrowArrayView* src,
+                                                       int64_t i,
+                                                       struct ArrowError* 
error) {
+  if (ArrowArrayViewIsNull(src, i)) {
+    return ArrowArrayAppendNull(dst, 1);
+  }
+
+  switch (src->storage_type) {
+    case NANOARROW_TYPE_NA:
+      return ArrowArrayAppendNull(dst, 1);
+    case NANOARROW_TYPE_BOOL:
+    case NANOARROW_TYPE_INT8:
+    case NANOARROW_TYPE_INT16:
+    case NANOARROW_TYPE_INT32:
+    case NANOARROW_TYPE_INT64:
+    case NANOARROW_TYPE_DATE32:
+    case NANOARROW_TYPE_DATE64:
+    case NANOARROW_TYPE_TIMESTAMP:
+    case NANOARROW_TYPE_TIME32:
+    case NANOARROW_TYPE_TIME64:
+    case NANOARROW_TYPE_DURATION:
+      return ArrowArrayAppendInt(dst, ArrowArrayViewGetIntUnsafe(src, i));
+    case NANOARROW_TYPE_UINT8:
+    case NANOARROW_TYPE_UINT16:
+    case NANOARROW_TYPE_UINT32:
+    case NANOARROW_TYPE_UINT64:
+      return ArrowArrayAppendUInt(dst, ArrowArrayViewGetUIntUnsafe(src, i));
+    case NANOARROW_TYPE_HALF_FLOAT:
+    case NANOARROW_TYPE_FLOAT:
+    case NANOARROW_TYPE_DOUBLE:
+      return ArrowArrayAppendDouble(dst, ArrowArrayViewGetDoubleUnsafe(src, 
i));
+    case NANOARROW_TYPE_STRING:
+    case NANOARROW_TYPE_BINARY:
+    case NANOARROW_TYPE_FIXED_SIZE_BINARY:
+    case NANOARROW_TYPE_LARGE_STRING:
+    case NANOARROW_TYPE_LARGE_BINARY:
+    case NANOARROW_TYPE_BINARY_VIEW:
+    case NANOARROW_TYPE_STRING_VIEW:
+      return ArrowArrayAppendBytes(dst, ArrowArrayViewGetBytesUnsafe(src, i));
+    case NANOARROW_TYPE_INTERVAL_MONTHS:
+    case NANOARROW_TYPE_INTERVAL_DAY_TIME:
+    case NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO: {
+      struct ArrowInterval interval;
+      ArrowIntervalInit(&interval, src->storage_type);
+      ArrowArrayViewGetIntervalUnsafe(src, i, &interval);
+      return ArrowArrayAppendInterval(dst, &interval);
+    }
+    case NANOARROW_TYPE_DECIMAL32:
+    case NANOARROW_TYPE_DECIMAL64:
+    case NANOARROW_TYPE_DECIMAL128:
+    case NANOARROW_TYPE_DECIMAL256: {
+      int32_t bitwidth = 32;
+      if (src->storage_type == NANOARROW_TYPE_DECIMAL64) {
+        bitwidth = 64;
+      } else if (src->storage_type == NANOARROW_TYPE_DECIMAL128) {
+        bitwidth = 128;
+      } else if (src->storage_type == NANOARROW_TYPE_DECIMAL256) {
+        bitwidth = 256;
+      }
+
+      struct ArrowDecimal decimal;
+      ArrowDecimalInit(&decimal, bitwidth, /*precision=*/0, /*scale=*/0);
+      ArrowArrayViewGetDecimalUnsafe(src, i, &decimal);
+      return ArrowArrayAppendDecimal(dst, &decimal);
+    }
+    case NANOARROW_TYPE_STRUCT:
+      for (int64_t child_i = 0; child_i < src->n_children; child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[child_i], src->children[child_i], src->offset + i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    case NANOARROW_TYPE_LIST:
+    case NANOARROW_TYPE_LARGE_LIST:
+    case NANOARROW_TYPE_MAP:
+    case NANOARROW_TYPE_LIST_VIEW:
+    case NANOARROW_TYPE_LARGE_LIST_VIEW: {
+      int64_t logical_i = src->offset + i;
+      int64_t child_offset = ArrowArrayViewListChildOffset(src, logical_i);
+      int64_t child_length;
+      if (src->storage_type == NANOARROW_TYPE_LIST_VIEW) {
+        child_length = src->buffer_views[2].data.as_int32[logical_i];
+      } else if (src->storage_type == NANOARROW_TYPE_LARGE_LIST_VIEW) {
+        child_length = src->buffer_views[2].data.as_int64[logical_i];
+      } else {
+        child_length = ArrowArrayViewListChildOffset(src, logical_i + 1) - 
child_offset;
+      }
+
+      for (int64_t child_i = 0; child_i < child_length; child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[0], src->children[0], child_offset + child_i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    }
+    case NANOARROW_TYPE_FIXED_SIZE_LIST: {
+      int64_t child_offset = (src->offset + i) * 
src->layout.child_size_elements;
+      for (int64_t child_i = 0; child_i < src->layout.child_size_elements; 
child_i++) {
+        NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+            dst->children[0], src->children[0], child_offset + child_i, 
error));
+      }
+      return ArrowArrayFinishElement(dst);
+    }
+    case NANOARROW_TYPE_DENSE_UNION:
+    case NANOARROW_TYPE_SPARSE_UNION: {
+      int8_t type_id = ArrowArrayViewUnionTypeId(src, i);
+      int8_t child_index = ArrowArrayViewUnionChildIndex(src, i);
+      int64_t child_offset = ArrowArrayViewUnionChildOffset(src, i);
+      NANOARROW_RETURN_NOT_OK(ArrowArrayAppendArrayViewElement(
+          dst->children[child_index], src->children[child_index], 
child_offset, error));
+      NANOARROW_RETURN_NOT_OK(
+          ArrowBufferAppend(ArrowArrayBuffer(dst, 0), &type_id, 
sizeof(type_id)));
+      if (src->storage_type == NANOARROW_TYPE_DENSE_UNION) {
+        _NANOARROW_CHECK_RANGE(dst->children[child_index]->length - 1, 0, 
INT32_MAX);
+        NANOARROW_RETURN_NOT_OK(ArrowBufferAppendInt32(
+            ArrowArrayBuffer(dst, 1), 
(int32_t)dst->children[child_index]->length - 1));
+      } else {
+        for (int64_t child_i = 0; child_i < dst->n_children; child_i++) {
+          if (child_i != child_index && dst->children[child_i]->length == 
dst->length) {
+            
NANOARROW_RETURN_NOT_OK(ArrowArrayAppendEmpty(dst->children[child_i], 1));
+          }
+          if (dst->children[child_i]->length != dst->length + 1) {
+            ArrowErrorSet(error,
+                          "Expected sparse union child length of %" PRId64
+                          " but found %" PRId64,
+                          dst->length + 1, dst->children[child_i]->length);
+            return EINVAL;
+          }
+        }
+      }
+      dst->length++;
+      return NANOARROW_OK;
+    }
+    case NANOARROW_TYPE_RUN_END_ENCODED:
+    case NANOARROW_TYPE_UNINITIALIZED:
+    default:
+      ArrowErrorSet(error, "Appending array views is not supported for %s",
+                    ArrowTypeString(src->storage_type));
+      return ENOTSUP;
+  }
+}
+
+ArrowErrorCode ArrowArrayAppendArrayView(struct ArrowArray* dst,
+                                         const struct ArrowArrayView* src,
+                                         struct ArrowError* error) {
+  if (src->storage_type == NANOARROW_TYPE_RUN_END_ENCODED) {
+    if (src->offset != 0) {
+      ArrowErrorSet(error, "Can't append a sliced run-end encoded array view");
+      return ENOTSUP;
+    }
+
+    int64_t run_end_offset = dst->length;
+    for (int64_t i = 0; i < src->children[0]->length; i++) {
+      NANOARROW_RETURN_NOT_OK(ArrowArrayAppendInt(
+          dst->children[0],
+          run_end_offset + ArrowArrayViewGetIntUnsafe(src->children[0], i)));

Review Comment:
   These (and below) should use `NANOARROW_RETURN_NOT_OK_WITH_ERROR()`...we are 
in general very careful about ensuring that the contents of `error` are 
populated if an error code is returned.



##########
src/nanoarrow/common/array_test.cc:
##########
@@ -5087,3 +5087,232 @@ TEST(ArrayMoveSharedTest, ArrayWithNullBuffers) {
 
   ArrowArrayRelease(&shared);
 }
+
+static ArrowErrorCode AppendArrayViewForTest(const struct ArrowArrayView* src,
+                                             struct ArrowArray* dst,
+                                             struct ArrowError* error) {

Review Comment:
   In addition to these lovely nanoarrow only tests, an Arrow C++ gated test I 
think would help cover some of the things that can happen here in a way that 
can be parameterized to cover more cases. Perhaps one test per type family 
(unigned int, signed int, strings, binaries, lists), testing against the Arrow 
C++ builder for the matrix of source and destination types? Arrow C++ makes 
asserting the equality a bit easier too.



-- 
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]

Reply via email to