gaborkaszab commented on code in PR #16425:
URL: https://github.com/apache/iceberg/pull/16425#discussion_r4538085721


##########
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`.
+
+See [Column file example](#column-file-example).
+
+When column files are added to a data file or a leaf manifest, the writer must 
produce a MODIFIED entry and a REPLACED entry as described in [Updating 
Tracking Metadata](#updating-tracking-metadata). The MODIFIED entry and the 
REPLACED entry may be written to different manifests.
+
+The `tracking` fields of the MODIFIED entry are set as described in [Updating 
Tracking Metadata](#updating-tracking-metadata) and [Sequence Number 
Inheritance](#sequence-number-inheritance), and `tracking.snapshot_id` must be 
the previous entry's (inherited) snapshot ID. The fields of the MODIFIED entry 
outside `tracking` must match the previous entry, except for the following:
+
+- `partition`:
+    - For a data file, an added column file's `field_ids` may have the ID of a 
partition source field, or of a field that contains one, only if all live rows 
share the same partition values after the update. `partition` must be set to 
those values.
+- `sort_order_id`:
+    - If an added column file lists a field that is, or contains, a field in 
the sort order, `sort_order_id` must be null.
+- `content_stats`:
+    - Stats for a field listed in an added column file, and for its nested 
fields, must be recomputed or omitted.
+    - For a leaf manifest, stats that summarize a listed field must be 
recomputed or omitted.
+- `tight_stats`:
+    - If `tight_stats` is true, `content_stats` must be tight for the live 
rows after the update (see [Stats Tightness](#stats-tightness)).
+- `deletion_vector`:
+    - For a data file, every delete that applied to it before the column files 
were added must be written into the MODIFIED entry's `deletion_vector`.
+- `column_files`:
+    - Each field listed in an added column file must be removed from the 
`field_ids` of the previous entry's column files. A column file with no 
remaining `field_ids` must be removed.

Review Comment:
   I'm wondering if we should include some specific rules when setting an 
entire struct and we already have column files for some of the fields of the 
struct.
   
   Following @RussellSpitzer 's example 
[here](https://github.com/apache/iceberg/pull/16425#issuecomment-6779153428), 
let's say we have a non-null `customer` struct in the base file, we also have a 
column file overriding `customer.name`, `CF-1[3]`, and then we run an update to 
override the entire `customer` struct setting it to null, introducing `CF-2[2]`.
   
   If I get the rules right, this is feasible. We don't say anything about 
removing `CF-1`, though, but we do have an overlapping column file after we 
commit `CF-2`.
   Shouldn't we say that if a struct's field ID is present in the field IDs of 
the new column file(s) then the field IDs of the (recursively collected) fields 
within the struct have to be removed from existing column files?
   
   The same question is not relevant when we set a struct from null to 
non-null, because we say we have to write all the members of that struct in the 
column file, and that in turn will trigger to remove those members from 
existing column files.



-- 
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]

Reply via email to