improve comments

Author: c-thiel

crates/iceberg/src/spec/manifest/data_file.rs

@@ -24,9 +24,10 @@ use serde_derive::{Deserialize, Serialize};
 use serde_with::{DeserializeFromStr, SerializeDisplay};
 
 use super::_serde::DataFileSerde;
-use super::{Datum, FormatVersion, Schema, data_file_schema_v1, data_file_schema_v2};
+use super::{
+    Datum, FormatVersion, Schema, data_file_schema_v1, data_file_schema_v2, data_file_schema_v3,
+};
 use crate::error::Result;
-use crate::spec::manifest::data_file_schema_v3;
 use crate::spec::{Struct, StructType};
 use crate::{Error, ErrorKind};
 
@@ -294,7 +295,6 @@ pub fn write_data_files_to_avro<W: Write>(
     let avro_schema = match version {
         FormatVersion::V1 => data_file_schema_v1(partition_type).unwrap(),
         FormatVersion::V2 => data_file_schema_v2(partition_type).unwrap(),
-        // Todo: Why unwrap here and not return the error?
         FormatVersion::V3 => data_file_schema_v3(partition_type).unwrap(),
     };
     let mut writer = AvroWriter::new(&avro_schema, writer);
@@ -323,7 +323,6 @@ pub fn read_data_files_from_avro<R: Read>(
     let avro_schema = match version {
         FormatVersion::V1 => data_file_schema_v1(partition_type).unwrap(),
         FormatVersion::V2 => data_file_schema_v2(partition_type).unwrap(),
-        // Todo: Why unwrap here and not return the error?
         FormatVersion::V3 => data_file_schema_v3(partition_type).unwrap(),
     };
 

crates/iceberg/src/spec/manifest/entry.rs

@@ -568,8 +568,9 @@ fn data_file_fields_v2(partition_type: &StructType) -> Vec<NestedFieldRef> {
         EQUALITY_IDS.clone(),
         SORT_ORDER_ID.clone(),
         FIRST_ROW_ID.clone(),
-        // Why are these three here - shouldn't they be v3 only?
         REFERENCE_DATA_FILE.clone(),
+        // Why are the following two fields here in the existing v2 schema?
+        // In the spec, they are not even listed as optional for v2.
         CONTENT_OFFSET.clone(),
         CONTENT_SIZE_IN_BYTES.clone(),
     ]