gaborkaszab commented on code in PR #16425: URL: https://github.com/apache/iceberg/pull/16425#discussion_r4537025942
########## format/spec.md: ########## @@ -1801,6 +1804,80 @@ If a delete column in an equality delete file is later dropped from the table, i Manifests hold the same statistics for delete files and data files. For delete files, the metrics describe the values that were deleted. +### Column Files + +A data file or leaf manifest may store fields in column files listed in [`column_files`](#column-file-fields). The data file or leaf manifest whose `column_files` lists a column file is that column file's _base file_. A column file can use any supported Iceberg file format and must have the same row count as its base file. + +A column file stores the values of the fields whose IDs are in its `field_ids`. A list or map is stored as a whole, including every field nested in it. For a struct, the column file stores whether the struct is null. Fields nested in the struct are stored only if their IDs are also in `field_ids`. Readers must ignore fields in a column file whose IDs are not in its `field_ids`. In this section, a field _contains_ every field nested in it, at any depth. + +When a column file is added, each ID in its `field_ids` must identify a field in the schema of its base file, or `_last_updated_sequence_number` (see [Writing Column Files](#writing-column-files)). The schema of a data file is the current table schema. The schema of a leaf manifest is the tracked file schema. + +#### Writing Column Files + +A column file stores a value for every row of its base file, including rows whose values did not change. Values for deleted rows, and for fields in a null struct, should be null. + +An added column file must follow these rules for `field_ids`: + +- A field nested in a list or map must not be in `field_ids`. When such a field changes, the list or map must be in `field_ids`. +- Each field whose value changes must be in `field_ids`. +- Each struct that changes between null and non-null must be in `field_ids`. +- Each field directly nested in a struct that has changed from null to non-null must be in `field_ids`. +- A field nested in a struct that is null after the update does not need to be in `field_ids`. Review Comment: Same comment here, "that is null after the update" part seems confusing to me if refers to the struct or the field in a struct. Maybe this? "If a struct is null after the update, its fields don't need to be in `field_ids`." ########## format/spec.md: ########## @@ -1801,6 +1804,80 @@ If a delete column in an equality delete file is later dropped from the table, i Manifests hold the same statistics for delete files and data files. For delete files, the metrics describe the values that were deleted. +### Column Files + +A data file or leaf manifest may store fields in column files listed in [`column_files`](#column-file-fields). The data file or leaf manifest whose `column_files` lists a column file is that column file's _base file_. A column file can use any supported Iceberg file format and must have the same row count as its base file. + +A column file stores the values of the fields whose IDs are in its `field_ids`. A list or map is stored as a whole, including every field nested in it. For a struct, the column file stores whether the struct is null. Fields nested in the struct are stored only if their IDs are also in `field_ids`. Readers must ignore fields in a column file whose IDs are not in its `field_ids`. In this section, a field _contains_ every field nested in it, at any depth. + +When a column file is added, each ID in its `field_ids` must identify a field in the schema of its base file, or `_last_updated_sequence_number` (see [Writing Column Files](#writing-column-files)). The schema of a data file is the current table schema. The schema of a leaf manifest is the tracked file schema. + +#### Writing Column Files + +A column file stores a value for every row of its base file, including rows whose values did not change. Values for deleted rows, and for fields in a null struct, should be null. + +An added column file must follow these rules for `field_ids`: + +- A field nested in a list or map must not be in `field_ids`. When such a field changes, the list or map must be in `field_ids`. +- Each field whose value changes must be in `field_ids`. +- Each struct that changes between null and non-null must be in `field_ids`. +- Each field directly nested in a struct that has changed from null to non-null must be in `field_ids`. Review Comment: I found it a bit confusing if "that has changed from null to nun-null" refers to the struct itself or to the fields. Probably it's a matter of having or not having a comma there, but maybe for non-native speakers like me it's not that obvious :) Instead can we do this: - Indent this line one level deeper - Say: "Each field directly nested in such a struct must be in `field_ids`" - Maybe also extend the sentence with "even if the field itself is null"? WDYT? -- 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]
