kacper-li-airspace-intelligence opened a new issue, #3445: URL: https://github.com/apache/iceberg-rust/issues/3445
### Apache Iceberg Rust version `main` (1270e329b) ### Describe the bug Java writes V1 manifest lists with the field names `added_files_count`, `existing_files_count` and `deleted_files_count` ([`V1Metadata.MANIFEST_LIST_SCHEMA`](https://github.com/apache/iceberg/blob/db43b88a6c0d470bd4f8acf08a18328750aeed20/core/src/main/java/org/apache/iceberg/V1Metadata.java#L32-L45), [`ManifestFile`](https://github.com/apache/iceberg/blob/db43b88a6c0d470bd4f8acf08a18328750aeed20/api/src/main/java/org/apache/iceberg/ManifestFile.java#L56-L61)), which are the names in the spec. `ManifestFileV1` and `MANIFEST_LIST_AVRO_SCHEMA_V1` only know the older `added_data_files_count` names, so `ManifestList::parse_with_version(_, FormatVersion::V1)` returns `None` for all three counts. Fields with the same name in both, such as `added_rows_count`, parse correctly. Reading the same bytes as V2 returns the counts, because `ManifestFileV2` has aliases for both names. The spec treats a null count as non-zero, so scan planning stays correct, but the counts are lost. `FastAppendAction::existing_manifest` copies these entries into the next manifest list, so a fast append on such a table writes the nulls back. ### To Reproduce Write one manifest list entry with Java's V1 field names and parse it: ```rust use apache_avro::types::Record; use apache_avro::{Schema, Writer}; use iceberg::spec::{FormatVersion, ManifestList}; let opt = |n: &str, t: &str| format!(r#"{{"name":"{n}","type":["null","{t}"],"default":null}}"#); let schema = Schema::parse_str(&format!( r#"{{"type":"record","name":"manifest_file","fields":[ {{"name":"manifest_path","type":"string"}},{{"name":"manifest_length","type":"long"}}, {{"name":"partition_spec_id","type":"int"}},{{"name":"added_snapshot_id","type":"long"}}, {},{},{},{},{},{},{}]}}"#, opt("added_files_count", "int"), opt("existing_files_count", "int"), opt("deleted_files_count", "int"), opt("added_rows_count", "long"), opt("existing_rows_count", "long"), opt("deleted_rows_count", "long"), opt("key_metadata", "bytes"), )).unwrap(); let mut writer = Writer::new(&schema, Vec::new()).unwrap(); let mut record = Record::new(&schema).unwrap(); record.put("manifest_path", "s3://t/m.avro"); record.put("manifest_length", 1i64); record.put("partition_spec_id", 0i32); record.put("added_snapshot_id", 1i64); record.put("added_files_count", Some(3i32)); record.put("existing_files_count", Some(0i32)); record.put("deleted_files_count", Some(0i32)); record.put("added_rows_count", Some(9i64)); record.put("existing_rows_count", Some(0i64)); record.put("deleted_rows_count", Some(0i64)); record.put("key_metadata", None::<Vec<u8>>); writer.append_value(record).unwrap(); let bs = writer.into_inner().unwrap(); let list = ManifestList::parse_with_version(&bs, FormatVersion::V1).unwrap(); let entry = &list.entries()[0]; // added_files_count, existing_files_count, deleted_files_count: None // added_rows_count: Some(9) ``` ### Expected behavior V1 parsing returns `Some(3)`, `Some(0)` and `Some(0)` for the three file counts. Once V1 reads against the writer schema (#3224), adding `#[serde(alias = ...)]` for `added_files_count`, `existing_files_count` and `deleted_files_count` to `ManifestFileV1`, as `ManifestFileV2` already does, fixes it. The fix should come with a test that uses Java's V1 names. ### Willingness to contribute I can contribute a fix for this bug independently ### Related - Reported in review of #3224: https://github.com/apache/iceberg-rust/pull/3224#discussion_r4259833708 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
