feat: make data types fields private

ffi/src/schema.rs

@@ -245,7 +245,7 @@ fn visit_schema_impl(schema: &StructType, visitor: &mut EngineSchemaVisitor) ->
         let metadata = CStringMap::default();
         visit_schema_item(
             "array_element",
-            &at.element_type,
+            at.element_type(),
             contains_null,
             &metadata,
             visitor,
@@ -263,15 +263,15 @@ fn visit_schema_impl(schema: &StructType, visitor: &mut EngineSchemaVisitor) ->
         let metadata = CStringMap::default();
         visit_schema_item(
             "map_key",
-            &mt.key_type,
+            mt.key_type(),
             false,
             &metadata,
             visitor,
             child_list_id,
         );
         visit_schema_item(
             "map_value",
-            &mt.value_type,
+            mt.value_type(),
             value_contains_null,
             &metadata,
             visitor,
@@ -306,11 +306,14 @@ fn visit_schema_impl(schema: &StructType, visitor: &mut EngineSchemaVisitor) ->
             DataType::Map(mt) => {
                 call!(
                     visit_map,
-                    visit_map_types(visitor, mt, mt.value_contains_null)
+                    visit_map_types(visitor, mt, mt.value_contains_null())
                 )
             }
             DataType::Array(at) => {
-                call!(visit_array, visit_array_item(visitor, at, at.contains_null))
+                call!(
+                    visit_array,
+                    visit_array_item(visitor, at, at.contains_null())
+                )
             }
             DataType::Primitive(PrimitiveType::Decimal(d)) => {
                 call!(visit_decimal, d.precision(), d.scale())

ffi/src/transaction/mod.rs

@@ -294,14 +294,22 @@ mod tests {
             let write_schema = unsafe { get_write_schema(write_context.shallow_copy()) };
             let write_schema_ref = unsafe { write_schema.as_ref() };
             assert_eq!(write_schema_ref.num_fields(), 2);
-            assert_eq!(write_schema_ref.field_at_index(0).unwrap().name, "number");
+            assert_eq!(write_schema_ref.field_at_index(0).unwrap().name(), "number");
             assert_eq!(
-                write_schema_ref.field_at_index(0).unwrap().data_type,
+                write_schema_ref
+                    .field_at_index(0)
+                    .unwrap()
+                    .data_type()
+                    .clone(),
                 DataType::INTEGER
             );
-            assert_eq!(write_schema_ref.field_at_index(1).unwrap().name, "string");
+            assert_eq!(write_schema_ref.field_at_index(1).unwrap().name(), "string");
             assert_eq!(
-                write_schema_ref.field_at_index(1).unwrap().data_type,
+                write_schema_ref
+                    .field_at_index(1)
+                    .unwrap()
+                    .data_type()
+                    .clone(),
                 DataType::STRING
             );
 

kernel/src/actions/mod.rs

@@ -264,7 +264,7 @@ impl Metadata {
         if let Some(metadata_field) = schema.fields().find(|field| field.is_metadata_column()) {
             return Err(Error::Schema(format!(
                 "Table schema must not contain metadata columns. Found metadata column: '{}'",
-                metadata_field.name
+                metadata_field.name()
             )));
         }
 

kernel/src/engine/arrow_expression/apply_schema.rs

@@ -74,9 +74,9 @@ fn transform_struct(
                 let target_field = target_field.borrow();
                 let transformed_col = apply_schema_to(&sa_col, target_field.data_type())?;
                 let transformed_field = new_field_with_metadata(
-                    &target_field.name,
+                    target_field.name(),
                     transformed_col.data_type(),
-                    target_field.nullable,
+                    target_field.is_nullable(),
                     Some(target_field.metadata_with_string_values()),
                 );
                 Ok((transformed_field, transformed_col))
@@ -118,11 +118,11 @@ fn apply_schema_to_list(
     };
     let (field, offset_buffer, values, nulls) = la.clone().into_parts();
 
-    let transformed_values = apply_schema_to(&values, &target_inner_type.element_type)?;
+    let transformed_values = apply_schema_to(&values, target_inner_type.element_type())?;
     let transformed_field = ArrowField::new(
         field.name(),
         transformed_values.data_type().clone(),
-        target_inner_type.contains_null,
+        target_inner_type.contains_null(),
     );
     Ok(ListArray::try_new(
         Arc::new(transformed_field),
@@ -143,8 +143,8 @@ fn apply_schema_to_map(array: &dyn Array, kernel_map_type: &MapType) -> DeltaRes
     let target_fields = map_struct_array
         .fields()
         .iter()
-        .zip([&kernel_map_type.key_type, &kernel_map_type.value_type])
-        .zip([false, kernel_map_type.value_contains_null])
+        .zip([kernel_map_type.key_type(), kernel_map_type.value_type()])
+        .zip([false, kernel_map_type.value_contains_null()])
         .map(|((arrow_field, target_type), nullable)| {
             StructField::new(arrow_field.name(), target_type.clone(), nullable)
         });

kernel/src/engine/arrow_expression/mod.rs

@@ -187,7 +187,7 @@ impl Scalar {
                 for _ in 0..num_rows {
                     let field_builders = builder.field_builders_mut().iter_mut();
                     for (builder, field) in field_builders.zip(stype.fields()) {
-                        Self::append_null(builder, &field.data_type, 1)?;
+                        Self::append_null(builder, field.data_type(), 1)?;
                     }
                     builder.append(false);
                 }

kernel/src/engine/arrow_utils.rs

@@ -431,13 +431,13 @@ fn get_indices(
         {
             // If the field is a variant, make sure the parquet schema matches the unshredded variant
             // representation. This is to ensure that shredded reads are not performed.
-            if requested_field.data_type == DataType::unshredded_variant() {
+            if requested_field.data_type().clone() == DataType::unshredded_variant() {
                 validate_parquet_variant(field)?;
             }
             match field.data_type() {
                 ArrowDataType::Struct(fields) => {
                     if let DataType::Struct(ref requested_schema)
-                    | DataType::Variant(ref requested_schema) = requested_field.data_type
+                    | DataType::Variant(ref requested_schema) = requested_field.data_type()
                     {
                         let (parquet_advance, children) = get_indices(
                             parquet_index + parquet_offset,
@@ -465,8 +465,8 @@ fn get_indices(
                     if let DataType::Array(array_type) = requested_field.data_type() {
                         let requested_schema = StructType::new_unchecked([StructField::new(
                             list_field.name().clone(), // so we find it in the inner call
-                            array_type.element_type.clone(),
-                            array_type.contains_null,
+                            array_type.element_type().clone(),
+                            array_type.contains_null(),
                         )]);
                         let (parquet_advance, mut children) = get_indices(
                             parquet_index + parquet_offset,
@@ -553,7 +553,7 @@ fn get_indices(
                     // parquet schema without causing issues in reading the data. We fix them up in
                     // expression evaluation later.
                     match super::ensure_data_types::ensure_data_types(
-                        &requested_field.data_type,
+                        requested_field.data_type(),
                         field.data_type(),
                         false,
                     )? {
@@ -600,7 +600,7 @@ fn get_indices(
                             "Metadata column {metadata_spec:?} is not supported by the default parquet reader"
                         )));
                     }
-                    None if field.nullable => {
+                    None if field.is_nullable() => {
                         debug!("Inserting missing and nullable field: {}", field.name());
                         reorder_indices.push(ReorderIndex::missing(
                             requested_position,

kernel/src/engine/ensure_data_types.rs

@@ -83,10 +83,10 @@ impl EnsureDataTypes {
             | (DataType::Array(inner_type), ArrowDataType::LargeListView(arrow_list_field)) => {
                 self.ensure_nullability(
                     "List",
-                    inner_type.contains_null,
+                    inner_type.contains_null(),
                     arrow_list_field.is_nullable(),
                 )?;
-                self.ensure_data_types(&inner_type.element_type, arrow_list_field.data_type())
+                self.ensure_data_types(inner_type.element_type(), arrow_list_field.data_type())
             }
             (DataType::Map(kernel_map_type), ArrowDataType::Map(arrow_map_type, _)) => {
                 let ArrowDataType::Struct(fields) = arrow_map_type.data_type() else {
@@ -97,13 +97,13 @@ impl EnsureDataTypes {
                         "Arrow map type didn't have expected key/value fields",
                     ));
                 };
-                self.ensure_data_types(&kernel_map_type.key_type, key_type.data_type())?;
+                self.ensure_data_types(kernel_map_type.key_type(), key_type.data_type())?;
                 self.ensure_nullability(
                     "Map",
-                    kernel_map_type.value_contains_null,
+                    kernel_map_type.value_contains_null(),
                     value_type.is_nullable(),
                 )?;
-                self.ensure_data_types(&kernel_map_type.value_type, value_type.data_type())?;
+                self.ensure_data_types(kernel_map_type.value_type(), value_type.data_type())?;
                 Ok(DataTypeCompat::Nested)
             }
             (DataType::Struct(kernel_fields), ArrowDataType::Struct(arrow_fields)) => {
@@ -117,7 +117,7 @@ impl EnsureDataTypes {
                 // ensure that for the fields that we found, the types match
                 for (kernel_field, arrow_field) in mapped_fields.zip(arrow_fields) {
                     self.ensure_nullability_and_metadata(kernel_field, arrow_field)?;
-                    self.ensure_data_types(&kernel_field.data_type, arrow_field.data_type())?;
+                    self.ensure_data_types(kernel_field.data_type(), arrow_field.data_type())?;
                     found_fields += 1;
                 }
 
@@ -165,17 +165,17 @@ impl EnsureDataTypes {
         arrow_field: &ArrowField,
     ) -> DeltaResult<()> {
         self.ensure_nullability(
-            &kernel_field.name,
-            kernel_field.nullable,
+            kernel_field.name(),
+            kernel_field.is_nullable(),
             arrow_field.is_nullable(),
         )?;
         if self.check_nullability_and_metadata
-            && !metadata_eq(&kernel_field.metadata, arrow_field.metadata())
+            && !metadata_eq(kernel_field.metadata(), arrow_field.metadata())
         {
             Err(Error::Generic(format!(
                 "Field {} has metadata {:?} in kernel and {:?} in arrow",
-                kernel_field.name,
-                kernel_field.metadata,
+                kernel_field.name(),
+                kernel_field.metadata(),
                 arrow_field.metadata(),
             )))
         } else {

kernel/src/expressions/scalars.rs

@@ -134,7 +134,7 @@ impl MapData {
                         v.data_type()
                     )))
                 // vals can only be null if value_contains_null is true
-                } else if v.is_null() && !data_type.value_contains_null {
+                } else if v.is_null() && !data_type.value_contains_null() {
                     Err(Error::schema(
                         "Null map value disallowed if map value_contains_null is false",
                     ))
@@ -396,7 +396,7 @@ impl Display for Scalar {
                 write!(f, "{{")?;
                 let mut delim = "";
                 for (value, field) in data.values.iter().zip(data.fields.iter()) {
-                    write!(f, "{delim}{}: {value}", field.name)?;
+                    write!(f, "{delim}{}: {value}", field.name())?;
                     delim = ", ";
                 }
                 write!(f, "}}")

kernel/src/scan/data_skipping.rs

@@ -85,14 +85,12 @@ impl DataSkippingFilter {
                 field: &'a StructField,
             ) -> Option<Cow<'a, StructField>> {
                 use Cow::*;
-                let field = match self.transform(&field.data_type)? {
+                let field = match self.transform(field.data_type())? {
                     Borrowed(_) if field.is_nullable() => Borrowed(field),
-                    data_type => Owned(StructField {
-                        name: field.name.clone(),
-                        data_type: data_type.into_owned(),
-                        nullable: true,
-                        metadata: field.metadata.clone(),
-                    }),
+                    data_type => Owned(
+                        StructField::new(field.name().clone(), data_type.into_owned(), true)
+                            .with_metadata(field.metadata().clone()),
+                    ),
                 };
                 Some(field)
             }

kernel/src/scan/mod.rs

@@ -241,7 +241,7 @@ impl<'a> SchemaTransform<'a> for GetReferencedFields<'a> {
 
     fn transform_struct_field(&mut self, field: &'a StructField) -> Option<Cow<'a, StructField>> {
         let physical_name = field.physical_name(self.column_mapping_mode);
-        self.logical_path.push(field.name.clone());
+        self.logical_path.push(field.name().clone());
         self.physical_path.push(physical_name.to_string());
         let field = self.recurse_into_struct_field(field);
         self.logical_path.pop();

kernel/src/scan/state_info.rs

@@ -174,7 +174,7 @@ impl StateInfo {
                         // if it's a normal physical column
                         let physical_field = logical_field.make_physical(column_mapping_mode);
                         debug!("\n\n{logical_field:#?}\nAfter mapping: {physical_field:#?}\n\n");
-                        let physical_name = physical_field.name.clone();
+                        let physical_name = physical_field.name().clone();
 
                         if !logical_field.is_metadata_column()
                             && metadata_info.metadata_field_names.contains(&physical_name)

kernel/src/schema/mod.rs

@@ -179,14 +179,14 @@ impl FromStr for MetadataColumnSpec {
 #[derive(Debug, Serialize, Deserialize, PartialEq, Clone, Eq)]
 pub struct StructField {
     /// Name of this (possibly nested) column
-    pub name: String,
+    name: String,
     /// The data type of this field
     #[serde(rename = "type")]
-    pub data_type: DataType,
+    data_type: DataType,
     /// Denotes whether this Field can be null
-    pub nullable: bool,
+    nullable: bool,
     /// A JSON map containing information about this column
-    pub metadata: HashMap<String, MetadataValue>,
+    metadata: HashMap<String, MetadataValue>,
 }
 
 impl StructField {
@@ -354,7 +354,7 @@ impl StructField {
     }
 
     #[inline]
-    pub fn is_nullable(&self) -> bool {
+    pub const fn is_nullable(&self) -> bool {
         self.nullable
     }
 
@@ -1144,11 +1144,11 @@ impl<'de> Deserialize<'de> for StructType {
 #[serde(rename_all = "camelCase")]
 pub struct ArrayType {
     #[serde(rename = "type")]
-    pub type_name: String,
+    type_name: String,
     /// The type of element stored in this array
-    pub element_type: DataType,
+    element_type: DataType,
     /// Denoting whether this array can contain one or more null values
-    pub contains_null: bool,
+    contains_null: bool,
 }
 
 impl ArrayType {
@@ -1175,14 +1175,14 @@ impl ArrayType {
 #[serde(rename_all = "camelCase")]
 pub struct MapType {
     #[serde(rename = "type")]
-    pub type_name: String,
+    type_name: String,
     /// The type of element used for the key of this map
-    pub key_type: DataType,
+    key_type: DataType,
     /// The type of element used for the value of this map
-    pub value_type: DataType,
+    value_type: DataType,
     /// Denoting whether this map can contain one or more null values
     #[serde(default = "default_true")]
-    pub value_contains_null: bool,
+    value_contains_null: bool,
 }
 
 impl MapType {

kernel/src/table_features/column_mapping.rs

@@ -81,7 +81,7 @@ impl<'a> ValidateColumnMappings<'a> {
         // The iterator yields `&&str` but `ColumnName::new` needs `&str`
         let column_name = || ColumnName::new(self.path.iter().copied());
         let annotation = "delta.columnMapping.physicalName";
-        match (self.mode, field.metadata.get(annotation)) {
+        match (self.mode, field.metadata().get(annotation)) {
             // Both Id and Name modes require a physical name annotation; None mode forbids it.
             (ColumnMappingMode::None, None) => {}
             (ColumnMappingMode::Name | ColumnMappingMode::Id, Some(MetadataValue::String(_))) => {}
@@ -106,7 +106,7 @@ impl<'a> ValidateColumnMappings<'a> {
         }
 
         let annotation = "delta.columnMapping.id";
-        match (self.mode, field.metadata.get(annotation)) {
+        match (self.mode, field.metadata().get(annotation)) {
             // Both Id and Name modes require a field ID annotation; None mode forbids it.
             (ColumnMappingMode::None, None) => {}
             (ColumnMappingMode::Name | ColumnMappingMode::Id, Some(MetadataValue::Number(_))) => {}
@@ -145,7 +145,7 @@ impl<'a> SchemaTransform<'a> for ValidateColumnMappings<'a> {
     }
     fn transform_struct_field(&mut self, field: &'a StructField) -> Option<Cow<'a, StructField>> {
         if self.err.is_none() {
-            self.path.push(&field.name);
+            self.path.push(field.name());
             self.check_annotations(field);
             let _ = self.recurse_into_struct_field(field);
             self.path.pop();

kernel/src/transaction/mod.rs

@@ -787,14 +787,12 @@ impl Transaction {
                 field: &'a StructField,
             ) -> Option<Cow<'a, StructField>> {
                 use Cow::*;
-                let field = match self.transform(&field.data_type)? {
+                let field = match self.transform(field.data_type())? {
                     Borrowed(_) if field.is_nullable() => Borrowed(field),
-                    data_type => Owned(StructField {
-                        name: field.name.clone(),
-                        data_type: data_type.into_owned(),
-                        nullable: true,
-                        metadata: field.metadata.clone(),
-                    }),
+                    data_type => Owned(
+                        StructField::new(field.name().clone(), data_type.into_owned(), true)
+                            .with_metadata(field.metadata().clone()),
+                    ),
                 };
                 Some(field)
             }