felipecrv commented on code in PR #35345:
URL: https://github.com/apache/arrow/pull/35345#discussion_r1388480798


##########
cpp/src/arrow/array/concatenate_test.cc:
##########
@@ -187,33 +189,89 @@ TEST_F(ConcatenateTest, FixedSizeListType) {
   });
 }
 
-TEST_F(ConcatenateTest, ListType) {
-  Check([this](int32_t size, double null_probability, std::shared_ptr<Array>* 
out) {
+template <typename ListType>
+struct ListConcatenationChecker {
+  using offset_type = typename ListType::offset_type;
+  using OffsetArrowType = typename CTypeTraits<offset_type>::ArrowType;
+  using ListArrayType = typename TypeTraits<ListType>::ArrayType;
+
+  template <typename Self>
+  static void Check(Self& self, int32_t size, double null_probability,
+                    std::shared_ptr<Array>* out) {
     auto values_size = size * 4;
-    auto values = this->GeneratePrimitive<Int8Type>(values_size, 
null_probability);
-    auto offsets_vector = this->Offsets<int32_t>(values_size, size);
+    auto values =
+        self.template GeneratePrimitive<Int8Type>(values_size, 
null_probability);
+    auto offsets_vector = self.template Offsets<offset_type>(values_size, 
size);
     // Ensure first and last offsets encompass the whole values array
     offsets_vector.front() = 0;
-    offsets_vector.back() = static_cast<int32_t>(values_size);
+    offsets_vector.back() = static_cast<offset_type>(values_size);
     std::shared_ptr<Array> offsets;
-    ArrayFromVector<Int32Type>(offsets_vector, &offsets);
-    ASSERT_OK_AND_ASSIGN(*out, ListArray::FromArrays(*offsets, *values));
+    ArrayFromVector<OffsetArrowType>(offsets_vector, &offsets);
+    ASSERT_OK_AND_ASSIGN(*out, ListArrayType::FromArrays(*offsets, *values));
     ASSERT_OK((**out).ValidateFull());
+  }
+};
+
+TEST_F(ConcatenateTest, ListType) {
+  Check([this](int32_t size, double null_probability, std::shared_ptr<Array>* 
out) {
+    ListConcatenationChecker<ListType>::Check(*this, size, null_probability, 
out);
   });
 }
 
 TEST_F(ConcatenateTest, LargeListType) {
   Check([this](int32_t size, double null_probability, std::shared_ptr<Array>* 
out) {
-    auto values_size = size * 4;
-    auto values = this->GeneratePrimitive<Int8Type>(values_size, 
null_probability);
-    auto offsets_vector = this->Offsets<int64_t>(values_size, size);
-    // Ensure first and last offsets encompass the whole values array
-    offsets_vector.front() = 0;
-    offsets_vector.back() = static_cast<int64_t>(values_size);
+    ListConcatenationChecker<LargeListType>::Check(*this, size, 
null_probability, out);
+  });
+}
+
+template <typename ListViewType>
+struct ListViewConcatenationChecker {
+  using offset_type = typename ListViewType::offset_type;
+  using OffsetArrowType = typename CTypeTraits<offset_type>::ArrowType;
+  using ListViewArrayType = typename TypeTraits<ListViewType>::ArrayType;
+
+  template <typename Self>
+  static void Check(Self& self, int32_t num_list_views, double 
null_probability,
+                    std::shared_ptr<Array>* out) {
+    auto values_size = 4 * num_list_views;
+    auto values =
+        self.template GeneratePrimitive<Int8Type>(values_size, 
null_probability);
+
     std::shared_ptr<Array> offsets;
-    ArrayFromVector<Int64Type>(offsets_vector, &offsets);
-    ASSERT_OK_AND_ASSIGN(*out, LargeListArray::FromArrays(*offsets, *values));
+    auto offsets_vector = self.template Offsets<offset_type>(values_size, 
num_list_views);
+    offsets_vector.front() = 0;

Review Comment:
   Instead of improving these generators I propose we use `testing/random.h`. I 
created an issue for this now -> https://github.com/apache/arrow/issues/38656
   
   But I would prefer tackling this problem in another PR. It's bigger than 
list-view: it would benefit the list concatenation tests as well.



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