alamb commented on code in PR #7487:
URL: https://github.com/apache/arrow-rs/pull/7487#discussion_r2082521310


##########
arrow-select/src/concat.rs:
##########
@@ -1267,4 +1339,123 @@ mod tests {
             "There are duplicates in the value list (the value list here is 
sorted which is only for the assertion)"
         );
     }
+
+    // Test the simple case of concatenating two RunArrays
+    #[test]
+    fn test_concat_run_array() {
+        // Create simple run arrays
+        let run_ends1 = Int32Array::from(vec![2, 4]);
+        let values1 = Int32Array::from(vec![10, 20]);
+        let array1 = RunArray::try_new(&run_ends1, &values1).unwrap();
+
+        let run_ends2 = Int32Array::from(vec![2, 4]);

Review Comment:
   Can you please change the run ends for the second array so they aren't the 
same as the first? Also it would be good to include a run with length 1
   
   Something like this perhaps:
   
   ```suggestion
           let run_ends2 = Int32Array::from(vec![1, 4]);
   ```



##########
arrow-select/src/concat.rs:
##########
@@ -1267,4 +1339,123 @@ mod tests {
             "There are duplicates in the value list (the value list here is 
sorted which is only for the assertion)"
         );
     }
+
+    // Test the simple case of concatenating two RunArrays
+    #[test]
+    fn test_concat_run_array() {
+        // Create simple run arrays
+        let run_ends1 = Int32Array::from(vec![2, 4]);
+        let values1 = Int32Array::from(vec![10, 20]);
+        let array1 = RunArray::try_new(&run_ends1, &values1).unwrap();
+
+        let run_ends2 = Int32Array::from(vec![2, 4]);
+        let values2 = Int32Array::from(vec![30, 40]);
+        let array2 = RunArray::try_new(&run_ends2, &values2).unwrap();
+
+        // Concatenate the arrays - this should now work properly
+        let result = concat(&[&array1, &array2]).unwrap();
+        let result_run_array = result
+            .as_any()
+            .downcast_ref::<RunArray<Int32Type>>()
+            .unwrap();
+
+        // Check that the result has the correct length
+        assert_eq!(result_run_array.len(), 8); // 4 + 4
+
+        // Check the run ends
+        let run_ends = result_run_array.run_ends().values();
+        assert_eq!(run_ends.len(), 4);
+        assert_eq!(&[2, 4, 6, 8], run_ends);
+
+        // Check the values
+        let values = result_run_array
+            .values()
+            .as_any()
+            .downcast_ref::<Int32Array>()
+            .unwrap();
+        assert_eq!(values.len(), 4);
+        assert_eq!(&[10, 20, 30, 40], values.values());
+    }
+
+    #[test]
+    fn test_concat_run_array_matching_first_last_value() {
+        // Create a run array with run ends [2, 4, 7] and values [10, 20, 30]
+        let run_ends1 = Int32Array::from(vec![2, 4, 7]);
+        let values1 = Int32Array::from(vec![10, 20, 30]);
+        let array1 = RunArray::try_new(&run_ends1, &values1).unwrap();
+
+        // Create another run array with run ends [3, 5] and values [30, 40]
+        let run_ends2 = Int32Array::from(vec![3, 5]);
+        let values2 = Int32Array::from(vec![30, 40]);
+        let array2 = RunArray::try_new(&run_ends2, &values2).unwrap();
+
+        // Concatenate the two arrays
+        let result = concat(&[&array1, &array2]).unwrap();
+        let result_run_array = result
+            .as_any()
+            .downcast_ref::<RunArray<Int32Type>>()
+            .unwrap();
+
+        // The result should have length 12 (7 + 5)
+        assert_eq!(result_run_array.len(), 12);
+
+        // Check that the run ends are correct
+        let run_ends = result_run_array.run_ends().values();
+        assert_eq!(&[2, 4, 7, 10, 12], run_ends);
+
+        // Check that the values are correct
+        assert_eq!(
+            &[10, 20, 30, 30, 40],
+            result_run_array
+                .values()
+                .as_any()
+                .downcast_ref::<Int32Array>()
+                .unwrap()
+                .values()
+        );
+    }
+
+    #[test]
+    fn test_concat_run_array_with_nulls() {

Review Comment:
   Can you also please add a test for: 
   * calling `concat` with 1 RunArray
   * Calling `concat` with 3 Run arrays
   



##########
arrow-select/src/concat.rs:
##########
@@ -1267,4 +1339,123 @@ mod tests {
             "There are duplicates in the value list (the value list here is 
sorted which is only for the assertion)"
         );
     }
+
+    // Test the simple case of concatenating two RunArrays
+    #[test]
+    fn test_concat_run_array() {
+        // Create simple run arrays
+        let run_ends1 = Int32Array::from(vec![2, 4]);
+        let values1 = Int32Array::from(vec![10, 20]);
+        let array1 = RunArray::try_new(&run_ends1, &values1).unwrap();
+
+        let run_ends2 = Int32Array::from(vec![2, 4]);
+        let values2 = Int32Array::from(vec![30, 40]);
+        let array2 = RunArray::try_new(&run_ends2, &values2).unwrap();
+
+        // Concatenate the arrays - this should now work properly
+        let result = concat(&[&array1, &array2]).unwrap();
+        let result_run_array = result

Review Comment:
   perhaps you could use the new `as_run()` methods added elsewhere in this PR



-- 
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-unsubscr...@arrow.apache.org

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

Reply via email to