Skip to content

V1 manifest lists written by Java lose added/existing/deleted file counts #3445

Open

Description

Apache Iceberg Rust version

main (1270e32)

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, ManifestFile), 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:

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions