[GitHub] [arrow] lidavidm commented on a diff in pull request #13009: ARROW-602: [C++] Provide iterator access to primitive elements inside an Array

[GitBox <gi...@apache.org>]

lidavidm commented on code in PR #13009:
URL: https://github.com/apache/arrow/pull/13009#discussion_r860854295


##########
cpp/src/arrow/chunked_array.h:
##########
@@ -141,6 +141,9 @@ class ARROW_EXPORT ChunkedArray {
   /// \brief Return the type of the chunked array
   const std::shared_ptr<DataType>& type() const { return type_; }
 
+  /// \brief Return chunk resolver of the chunked array
+  const internal::ChunkResolver& GetChunkResolver() const { return chunk_resolver_; }

Review Comment:
   At the very least it needs to be clearly marked off as an internal API, right now it looks like any other public API. Ideally it would be private with a friend declaration.
   
   ```suggestion
     const internal::ChunkResolver& chunk_resolver() const { return chunk_resolver_; }
   ```



##########
cpp/src/arrow/stl_iterator_test.cc:
##########
@@ -248,5 +248,216 @@ TEST(ArrayIterator, StdMerge) {
   ASSERT_EQ(values, expected);
 }
 
+TEST(ChunkedArrayIterator, Basics) {
+  auto chunk0 = ArrayFromJSON(int32(), "[4, 5, null]");
+  auto chunk1 = ArrayFromJSON(int32(), "[6]");
+
+  ASSERT_OK_AND_ASSIGN(auto result, ChunkedArray::Make({chunk0, chunk1}, int32()));

Review Comment:
   Note there's a `ChunkedArrayFromJSON`.
   
   Can we also test a chunked array with no chunks?



##########
cpp/src/arrow/stl_iterator_test.cc:
##########
@@ -248,5 +248,216 @@ TEST(ArrayIterator, StdMerge) {
   ASSERT_EQ(values, expected);
 }
 
+TEST(ChunkedArrayIterator, Basics) {
+  auto chunk0 = ArrayFromJSON(int32(), "[4, 5, null]");
+  auto chunk1 = ArrayFromJSON(int32(), "[6]");
+
+  ASSERT_OK_AND_ASSIGN(auto result, ChunkedArray::Make({chunk0, chunk1}, int32()));
+  auto it = Iterate<Int32Type>(*result);
+  optional<int32_t> v = *it;
+  ASSERT_EQ(v, 4);
+  ASSERT_EQ(it[0], 4);
+  ++it;
+  ASSERT_EQ(it[0], 5);
+  ASSERT_EQ(*it, 5);
+  ASSERT_EQ(it[1], nullopt);
+  ASSERT_EQ(it[2], 6);

Review Comment:
   Is indexing an iterator a typical operation?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org