Merge remote-tracking branch 'origin/develop' into aduffy/fuzz2

Author: a10y

Cargo.lock

@@ -38,6 +38,9 @@ tokio-stream = { workspace = true }
 vortex = { workspace = true, features = ["files"] }
 vortex-file = { workspace = true, features = ["tokio"] }
 
+[dev-dependencies]
+rstest = { workspace = true }
+
 [lints]
 workspace = true
 

vortex-duckdb/Cargo.toml

@@ -319,6 +319,7 @@ impl TryFrom<&DType> for LogicalType {
 mod tests {
     use std::sync::Arc;
 
+    use rstest::rstest;
     use vortex::dtype::{DType, FieldName, FieldNames, Nullability, PType, StructFields};
 
     use crate::cpp;
@@ -344,39 +345,21 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_integer_types() {
-        // Test signed and unsigned integers.
-        let test_cases = [
-            (PType::I8, cpp::DUCKDB_TYPE::DUCKDB_TYPE_TINYINT),
-            (PType::I16, cpp::DUCKDB_TYPE::DUCKDB_TYPE_SMALLINT),
-            (PType::I32, cpp::DUCKDB_TYPE::DUCKDB_TYPE_INTEGER),
-            (PType::I64, cpp::DUCKDB_TYPE::DUCKDB_TYPE_BIGINT),
-            (PType::U8, cpp::DUCKDB_TYPE::DUCKDB_TYPE_UTINYINT),
-            (PType::U16, cpp::DUCKDB_TYPE::DUCKDB_TYPE_USMALLINT),
-            (PType::U32, cpp::DUCKDB_TYPE::DUCKDB_TYPE_UINTEGER),
-            (PType::U64, cpp::DUCKDB_TYPE::DUCKDB_TYPE_UBIGINT),
-        ];
-
-        for (ptype, expected_duckdb_type) in test_cases {
-            let dtype = DType::Primitive(ptype, Nullability::NonNullable);
-            let logical_type = LogicalType::try_from(&dtype).unwrap();
-            assert_eq!(logical_type.as_type_id(), expected_duckdb_type);
-        }
-    }
-
-    #[test]
-    fn test_float_types() {
-        let float_test_cases = [
-            (PType::F32, cpp::DUCKDB_TYPE::DUCKDB_TYPE_FLOAT),
-            (PType::F64, cpp::DUCKDB_TYPE::DUCKDB_TYPE_DOUBLE),
-        ];
-
-        for (ptype, expected_duckdb_type) in float_test_cases {
-            let dtype = DType::Primitive(ptype, Nullability::NonNullable);
-            let logical_type = LogicalType::try_from(&dtype).unwrap();
-            assert_eq!(logical_type.as_type_id(), expected_duckdb_type);
-        }
+    #[rstest]
+    #[case(PType::I8, cpp::DUCKDB_TYPE::DUCKDB_TYPE_TINYINT)]
+    #[case(PType::I16, cpp::DUCKDB_TYPE::DUCKDB_TYPE_SMALLINT)]
+    #[case(PType::I32, cpp::DUCKDB_TYPE::DUCKDB_TYPE_INTEGER)]
+    #[case(PType::I64, cpp::DUCKDB_TYPE::DUCKDB_TYPE_BIGINT)]
+    #[case(PType::U8, cpp::DUCKDB_TYPE::DUCKDB_TYPE_UTINYINT)]
+    #[case(PType::U16, cpp::DUCKDB_TYPE::DUCKDB_TYPE_USMALLINT)]
+    #[case(PType::U32, cpp::DUCKDB_TYPE::DUCKDB_TYPE_UINTEGER)]
+    #[case(PType::U64, cpp::DUCKDB_TYPE::DUCKDB_TYPE_UBIGINT)]
+    #[case(PType::F32, cpp::DUCKDB_TYPE::DUCKDB_TYPE_FLOAT)]
+    #[case(PType::F64, cpp::DUCKDB_TYPE::DUCKDB_TYPE_DOUBLE)]
+    fn test_primitive_types(#[case] ptype: PType, #[case] expected_duckdb_type: cpp::DUCKDB_TYPE) {
+        let dtype = DType::Primitive(ptype, Nullability::NonNullable);
+        let logical_type = LogicalType::try_from(&dtype).unwrap();
+        assert_eq!(logical_type.as_type_id(), expected_duckdb_type);
     }
 
     #[test]

vortex-duckdb/src/convert/dtype.rs

@@ -145,12 +145,13 @@ impl ZonedLayout {
         usize::try_from(self.zones.row_count()).vortex_expect("Invalid number of zones")
     }
 
+    /// Returns an array of stats that exist in the layout's data, must be sorted.
     pub fn present_stats(&self) -> &Arc<[Stat]> {
         &self.present_stats
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, PartialEq, Eq, Clone)]
 pub struct ZonedMetadata {
     pub(super) zone_len: u32,
     pub(super) present_stats: Arc<[Stat]>,
@@ -179,3 +180,50 @@ impl SerializeMetadata for ZonedMetadata {
         metadata
     }
 }
+
+#[cfg(test)]
+mod tests {
+
+    use rstest::rstest;
+
+    use super::*;
+
+    #[rstest]
+    #[case(ZonedMetadata {
+            zone_len: u32::MAX,
+            present_stats: Arc::new([]),
+        })]
+    #[case(ZonedMetadata {
+            zone_len: 0,
+            present_stats: Arc::new([Stat::IsConstant]),
+        })]
+    #[case::all_sorted(ZonedMetadata {
+            zone_len: 314,
+            present_stats: Arc::new([Stat::IsConstant, Stat::IsSorted, Stat::IsStrictSorted, Stat::Max, Stat::Min, Stat::Sum, Stat::NullCount, Stat::UncompressedSizeInBytes, Stat::NaNCount]),
+        })]
+    #[case::some_sorted(ZonedMetadata {
+            zone_len: 314,
+            present_stats: Arc::new([Stat::IsSorted, Stat::IsStrictSorted, Stat::Max, Stat::Min, Stat::Sum, Stat::NullCount, Stat::UncompressedSizeInBytes, Stat::NaNCount]),
+        })]
+    fn test_metadata_serialization(#[case] metadata: ZonedMetadata) {
+        let serialized = metadata.clone().serialize();
+        let deserialized = ZonedMetadata::deserialize(&serialized).unwrap();
+        assert_eq!(deserialized, metadata);
+    }
+
+    #[test]
+    fn test_deserialize_unsorted_stats() {
+        let metadata = ZonedMetadata {
+            zone_len: u32::MAX,
+            present_stats: Arc::new([Stat::IsStrictSorted, Stat::IsSorted]),
+        };
+        let serialized = metadata.clone().serialize();
+        let deserialized = ZonedMetadata::deserialize(&serialized).unwrap();
+        assert!(deserialized.present_stats.is_sorted());
+        assert_eq!(
+            deserialized.present_stats.len(),
+            metadata.present_stats.len()
+        );
+        assert_ne!(deserialized.present_stats, metadata.present_stats);
+    }
+}

vortex-layout/src/layouts/zoned/mod.rs

@@ -31,8 +31,8 @@ pub struct ZoneMap {
 }
 
 impl ZoneMap {
-    /// Create StatsTable of given column_dtype from given array. Validates that the array matches expected
-    /// structure for given list of stats
+    /// Create [`ZoneMap`] of given column_dtype from given array. Validates that the array matches expected
+    /// structure for given list of stats.
     pub fn try_new(
         column_dtype: DType,
         array: StructArray,
@@ -45,12 +45,12 @@ impl ZoneMap {
         Ok(Self::new_unchecked(array, stats))
     }
 
-    /// Create StatsTable without validating return array against expected stats
+    /// Creates [`ZoneMap`] without validating return array against expected stats.
     pub fn new_unchecked(array: StructArray, stats: Arc<[Stat]>) -> Self {
         Self { array, stats }
     }
 
-    /// Returns the DType of the statistics table given a set of statistics and column [`DType`].
+    /// Returns the [`DType`] of the statistics table given a set of statistics and column [`DType`].
     pub fn dtype_for_stats_table(column_dtype: &DType, present_stats: &[Stat]) -> DType {
         assert!(present_stats.is_sorted(), "Stats must be sorted");
         DType::Struct(
@@ -77,17 +77,17 @@ impl ZoneMap {
         )
     }
 
-    /// The struct array backing the zone map
+    /// Returns the underlying [`StructArray`] backing the zone map
     pub fn array(&self) -> &StructArray {
         &self.array
     }
 
-    /// The statistics that are included in the table.
+    /// Returns the list of stats included in the zone map.
     pub fn present_stats(&self) -> &Arc<[Stat]> {
         &self.stats
     }
 
-    /// Return an aggregated stats set for the table.
+    /// Returns an aggregated stats set for the table.
     pub fn to_stats_set(&self, stats: &[Stat]) -> VortexResult<StatsSet> {
         let mut stats_set = StatsSet::default();
         for &stat in stats {
@@ -117,7 +117,7 @@ impl ZoneMap {
         Ok(stats_set)
     }
 
-    /// Return the array for a given stat.
+    /// Returns the array for a given stat.
     pub fn get_stat(&self, stat: Stat) -> VortexResult<Option<ArrayRef>> {
         Ok(self.array.field_by_name_opt(stat.name()).cloned())
     }
@@ -139,12 +139,11 @@ impl ZoneMap {
     }
 }
 
+// TODO(ngates): we should make it such that the zone map stores a mirror of the DType
+//  underneath each stats column. For example, `min: i32` for an `i32` array.
+//  Or `min: {a: i32, b: i32}` for a struct array of type `{a: i32, b: i32}`.
+//  See: <https://github.com/vortex-data/vortex/issues/1835>
 /// Accumulates statistics for a column.
-///
-/// TODO(ngates): we should make it such that the zone map stores a mirror of the DType
-///  underneath each stats column. For example, `min: i32` for an `i32` array.
-///  Or `min: {a: i32, b: i32}` for a struct array of type `{a: i32, b: i32}`.
-///  See: <https://github.com/vortex-data/vortex/issues/1835>
 pub struct StatsAccumulator {
     builders: Vec<Box<dyn StatsArrayBuilder>>,
     length: usize,