fix: Correctly canonicalize nullable list arrays (#2808)

Author: robert3005

encodings/fsst/src/canonical.rs

@@ -164,7 +164,7 @@ mod tests {
         let (chunked_arr, data) = make_data_chunked();
 
         let mut builder =
-            VarBinViewBuilder::with_capacity(DType::Utf8(Nullability::Nullable), chunked_arr.len());
+            VarBinViewBuilder::with_capacity(chunked_arr.dtype().clone(), chunked_arr.len());
         chunked_arr.append_to_builder(&mut builder).unwrap();
 
         {

vortex-array/src/array/implementation.rs

@@ -186,21 +186,21 @@ impl<A: ArrayImpl + 'static> Array for A {
     ///
     /// The [`DType`] of the builder must match that of the array.
     fn append_to_builder(&self, builder: &mut dyn ArrayBuilder) -> VortexResult<()> {
-        // TODO(ngates): add dtype function to ArrayBuilder
-        // if builder.dtype() != self.dtype() {
-        //     vortex_bail!(
-        //         "Builder dtype mismatch: expected {:?}, got {:?}",
-        //         self.dtype(),
-        //         builder.dtype()
-        //     );
-        // }
+        if builder.dtype() != self.dtype() {
+            vortex_bail!(
+                "Builder dtype mismatch: expected {}, got {}",
+                self.dtype(),
+                builder.dtype(),
+            );
+        }
         let len = builder.len();
 
         ArrayCanonicalImpl::_append_to_builder(self, builder)?;
         assert_eq!(
             len + self.len(),
             builder.len(),
-            "Builder length mismatch after writing array"
+            "Builder length mismatch after writing array for encoding {}",
+            self.encoding(),
         );
         Ok(())
     }

vortex-array/src/arrays/constant/canonical.rs

@@ -145,13 +145,13 @@ fn canonical_list_array(
 ) -> VortexResult<ListArray> {
     match values {
         None => ListArray::try_new(
-            ConstantArray::new(Scalar::null(element_dtype.clone()), 1).into_array(),
+            Canonical::empty(element_dtype).into_array(),
             ConstantArray::new(
                 Scalar::new(
                     DType::Primitive(PType::U64, Nullability::NonNullable),
                     ScalarValue::from(0),
                 ),
-                len,
+                len + 1,
             )
             .into_array(),
             Validity::AllInvalid,
@@ -305,4 +305,29 @@ mod tests {
             [0u64, 0, 0]
         );
     }
+
+    #[test]
+    fn test_canonicalize_null_list() {
+        let list_scalar = Scalar::null(DType::List(
+            Arc::new(DType::Primitive(PType::U64, Nullability::NonNullable)),
+            Nullability::Nullable,
+        ));
+        let const_array = ConstantArray::new(list_scalar, 2).into_array();
+        let canonical_const = const_array.to_list().unwrap();
+        assert!(
+            canonical_const
+                .elements()
+                .to_primitive()
+                .unwrap()
+                .is_empty()
+        );
+        assert_eq!(
+            canonical_const
+                .offsets()
+                .to_primitive()
+                .unwrap()
+                .as_slice::<u64>(),
+            [0u64, 0, 0]
+        );
+    }
 }