fix incorrect child vector size in duckdb list array import

Signed-off-by: Connor Tsui <[email protected]>

Author: connortsui20

vortex-duckdb/src/convert/vector.rs

@@ -222,26 +222,44 @@ pub fn flat_vector_to_vortex(vector: &mut Vector, len: usize) -> VortexResult<Ar
         }
         DUCKDB_TYPE::DUCKDB_TYPE_LIST => {
             let mut offsets = BufferMut::with_capacity(len);
-            let mut lengths = BufferMut::with_capacity(len);
-            for duckdb_list_entry { offset, length } in
-                vector.as_slice_with_len::<duckdb_list_entry>(len)
+            let mut sizes = BufferMut::with_capacity(len);
+
+            let mut child_max_length = 0;
+            for duckdb_list_entry {
+                offset,
+                length: size,
+            } in vector.as_slice_with_len::<duckdb_list_entry>(len)
             {
+                let offset = i64::try_from(*offset).vortex_expect("offset must fit i64");
+                assert!(offset >= 0, "offset must be non-negative");
+                let size = i64::try_from(*size).vortex_expect("length must fit i64");
+                assert!(size >= 0, "size must be non-negative");
+
+                // TODO(connor): Figure out if we need to worry about validity. In Arrow, null list
+                // views must still have a valid offset and size, but this might not be true is
+                // DuckDB? The public docs don't say anything about this...
+
+                // We have to track the ends of all list views to determine the length of the child
+                // vector since 1) they can be arbitrarily out of order and 2) there might be a null
+                // list at the end of the vector.
+                let end = usize::try_from(offset + size).vortex_expect(
+                    "either math is broken or the child vector length did not \
+                    fit into a 32-bit `usize` type",
+                );
+                child_max_length = child_max_length.max(end);
+
+                // SAFETY: We just allocated enough capacity for both of these.
                 unsafe {
-                    offsets.push_unchecked(
-                        i64::try_from(*offset).vortex_expect("offset must fit i64"),
-                    );
-                    lengths.push_unchecked(
-                        i64::try_from(*length).vortex_expect("length must fit i64"),
-                    );
+                    offsets.push_unchecked(offset);
+                    sizes.push_unchecked(size);
                 }
             }
+
             let offsets = offsets.freeze();
-            let lengths = lengths.freeze();
-            let child_data = flat_vector_to_vortex(
-                &mut vector.list_vector_get_child(),
-                usize::try_from(offsets[len - 1] + lengths[len - 1])
-                    .vortex_expect("last offset and length sum must fit in usize "),
-            )?;
+            let lengths = sizes.freeze();
+
+            let child_data =
+                flat_vector_to_vortex(&mut vector.list_vector_get_child(), child_max_length)?;
 
             ListViewArray::try_new(
                 child_data,
@@ -651,4 +669,105 @@ mod tests {
             &[5, 6, 7, 8]
         );
     }
+
+    #[test]
+    fn test_list_with_trailing_null() {
+        // Regression test: when the last list entry is null, its offset/length may be 0/0,
+        // so we can't use the last entry to compute child vector length.
+        let child_values = vec![1i32, 2, 3, 4];
+        let len = 2;
+
+        let logical_type =
+            LogicalType::list_type(LogicalType::new(DUCKDB_TYPE::DUCKDB_TYPE_INTEGER))
+                .vortex_unwrap();
+        let mut vector = Vector::with_capacity(logical_type, len);
+
+        // Entry 0: offset=0, length=4 -> all elements (end=4)
+        // Entry 1: null, offset=0, length=0 (end=0)
+        unsafe {
+            let entries = vector.as_slice_mut::<duckdb_list_entry>(len);
+            entries[0] = duckdb_list_entry {
+                offset: 0,
+                length: child_values.len() as u64,
+            };
+            entries[1] = duckdb_list_entry {
+                offset: 0,
+                length: 0,
+            };
+            let mut child = vector.list_vector_get_child();
+            let slice = child.as_slice_mut::<i32>(child_values.len());
+            slice.copy_from_slice(&child_values);
+        }
+
+        // Set the second entry as null.
+        let validity_slice = unsafe { vector.ensure_validity_bitslice(len) };
+        validity_slice.set(1, false);
+
+        // Test conversion - the old bug would compute child length as 0+0=0 instead of
+        // max(4,0)=4.
+        let result = flat_vector_to_vortex(&mut vector, len).unwrap();
+        let vortex_array = result.to_listview();
+
+        assert_eq!(vortex_array.len(), len);
+        assert_eq!(
+            vortex_array
+                .list_elements_at(0)
+                .to_primitive()
+                .as_slice::<i32>(),
+            &[1, 2, 3, 4]
+        );
+        assert_eq!(vortex_array.validity_mask(), Mask::from_indices(2, vec![0]));
+    }
+
+    #[test]
+    fn test_list_out_of_order() {
+        // Regression test: list views can be out of order in DuckDB. The child vector length
+        // must be computed as the maximum end offset, not just the last entry's end offset.
+        let child_values = vec![1i32, 2, 3, 4];
+        let len = 2;
+
+        let logical_type =
+            LogicalType::list_type(LogicalType::new(DUCKDB_TYPE::DUCKDB_TYPE_INTEGER))
+                .vortex_unwrap();
+        let mut vector = Vector::with_capacity(logical_type, len);
+
+        // Populate with out-of-order list entries:
+        // - Entry 0: offset=2, length=2 -> elements [3, 4] (end=4)
+        // - Entry 1: offset=0, length=2 -> elements [1, 2] (end=2)
+        unsafe {
+            let entries = vector.as_slice_mut::<duckdb_list_entry>(len);
+            entries[0] = duckdb_list_entry {
+                offset: 2,
+                length: 2,
+            };
+            entries[1] = duckdb_list_entry {
+                offset: 0,
+                length: 2,
+            };
+            let mut child = vector.list_vector_get_child();
+            let slice = child.as_slice_mut::<i32>(child_values.len());
+            slice.copy_from_slice(&child_values);
+        }
+
+        // Test conversion - the old bug would compute child length as 0+2=2 instead of
+        // max(4,2)=4.
+        let result = flat_vector_to_vortex(&mut vector, len).unwrap();
+        let vortex_array = result.to_listview();
+
+        assert_eq!(vortex_array.len(), len);
+        assert_eq!(
+            vortex_array
+                .list_elements_at(0)
+                .to_primitive()
+                .as_slice::<i32>(),
+            &[3, 4]
+        );
+        assert_eq!(
+            vortex_array
+                .list_elements_at(1)
+                .to_primitive()
+                .as_slice::<i32>(),
+            &[1, 2]
+        );
+    }
 }