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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to