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


##########
format/spec.md:
##########
@@ -1801,6 +1804,44 @@ 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
+
+Starting in v4, a data file or v4 leaf manifest may store fields in column 
files listed in [`column_files`](#column-file-fields). A column file can use 
any supported Iceberg file format and must have rows that align with the file 
it modifies.
+
+A column file stores the fields listed in its `field_ids`, each as a whole, 
including all of its nested fields. Each ID must identify a field in the schema 
of the file it modifies, which is the table schema for a data file and the 
tracked file schema for a leaf manifest, or a row lineage field required by 
[Writing Column Files](#writing-column-files). Fields nested within a list or 
map must not be listed in `field_ids`.
+
+#### Writing Column Files
+
+The rules below apply to column files of data files. Rules for writing column 
files of leaf manifests are not defined here.

Review Comment:
   I would think the leaf manifest rules are the same. maybe instead of saying 
`data file`, should we just say `base file`?



##########
format/spec.md:
##########
@@ -1801,6 +1804,44 @@ 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
+
+Starting in v4, a data file or v4 leaf manifest may store fields in column 
files listed in [`column_files`](#column-file-fields). A column file can use 
any supported Iceberg file format and must have rows that align with the file 
it modifies.
+
+A column file stores the fields listed in its `field_ids`, each as a whole, 
including all of its nested fields. Each ID must identify a field in the schema 
of the file it modifies, which is the table schema for a data file and the 
tracked file schema for a leaf manifest, or a row lineage field required by 
[Writing Column Files](#writing-column-files). Fields nested within a list or 
map must not be listed in `field_ids`.
+
+#### Writing Column Files
+
+The rules below apply to column files of data files. Rules for writing column 
files of leaf manifests are not defined here.
+
+A column file stores the value of each of its listed fields for every row of 
the data file. Values that did not change are still included, and values for 
deleted rows should be null.
+
+When column files are added to a data file, 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 other fields of the MODIFIED 
entry must match the previous entry, except for the following:
+
+- `partition`:
+    - If an added column file lists a field that is, or contains, a source 
field of the data file's partition spec, every row that is not deleted must 
have the same partition values after the update, and `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 computed from that column file, or omitted.
+- `tight_stats`:
+    - `tight_stats` must be false or omitted.
+- `deletion_vector`:
+    - Every delete that applied to the data file before the column files were 
added must be written into the MODIFIED entry's `deletion_vector`.
+- `column_files`:
+    - Each field ID listed in an added column file must be removed from the 
`field_ids` of the previous entry's column files, and a column file with no 
remaining `field_ids` must be removed.
+    - One of the added column files must take responsibility for 
`_last_updated_sequence_number` by listing it in its `field_ids`, replacing the 
data file or previous column file that stored it. In that column file:
+        - Rows the update changed must store null, so they inherit the 
MODIFIED entry's data sequence number.
+        - Other rows that are not deleted must store the non-null 
`_last_updated_sequence_number` read from the previous entry.
+
+#### Reading Column Files
+
+A projected field whose ID is in a column file's `field_ids` must be read from 
that column file. All other projected fields are read from the file it 
modifies, using [Column Projection](#column-projection) for a data file.
+
+A column file and the file it modifies are aligned by row position before 
deletes are applied from the data file's `deletion_vector` or the leaf 
manifest's `manifest_info.dv`. The scan schema must be used for field types and 
nullability. Null values in a column file may represent deleted rows.

Review Comment:
   > before deletes are applied from the data file's `deletion_vector` or the 
leaf manifest's `manifest_info.dv`
   
   might be easier to just say `before attached deletion vector is applied`



##########
format/spec.md:
##########
@@ -1801,6 +1804,44 @@ 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
+
+Starting in v4, a data file or v4 leaf manifest may store fields in column 
files listed in [`column_files`](#column-file-fields). A column file can use 
any supported Iceberg file format and must have rows that align with the file 
it modifies.
+
+A column file stores the fields listed in its `field_ids`, each as a whole, 
including all of its nested fields. Each ID must identify a field in the schema 
of the file it modifies, which is the table schema for a data file and the 
tracked file schema for a leaf manifest, or a row lineage field required by 
[Writing Column Files](#writing-column-files). Fields nested within a list or 
map must not be listed in `field_ids`.
+
+#### Writing Column Files
+
+The rules below apply to column files of data files. Rules for writing column 
files of leaf manifests are not defined here.
+
+A column file stores the value of each of its listed fields for every row of 
the data file. Values that did not change are still included, and values for 
deleted rows should be null.
+
+When column files are added to a data file, 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 other fields of the MODIFIED 
entry must match the previous entry, except for the following:
+
+- `partition`:
+    - If an added column file lists a field that is, or contains, a source 
field of the data file's partition spec, every row that is not deleted must 
have the same partition values after the update, and `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 computed from that column file, or omitted.
+- `tight_stats`:
+    - `tight_stats` must be false or omitted.
+- `deletion_vector`:
+    - Every delete that applied to the data file before the column files were 
added must be written into the MODIFIED entry's `deletion_vector`.
+- `column_files`:
+    - Each field ID listed in an added column file must be removed from the 
`field_ids` of the previous entry's column files, and a column file with no 
remaining `field_ids` must be removed.
+    - One of the added column files must take responsibility for 
`_last_updated_sequence_number` by listing it in its `field_ids`, replacing the 
data file or previous column file that stored it. In that column file:
+        - Rows the update changed must store null, so they inherit the 
MODIFIED entry's data sequence number.
+        - Other rows that are not deleted must store the non-null 
`_last_updated_sequence_number` read from the previous entry.

Review Comment:
   Follow the v3 row-lineage framing: a changed row stores null so the new 
sequence number replaces the current value, and an unchanged row copies the 
existing value. A column file has one extra case. The previous entry can store 
null, which means inherit, so that null is resolved before copying.
   
   ```suggestion
           - If the added column files change the row, 
`_last_updated_sequence_number` must be set to `null` (so that the sequence 
number of the snapshot that added the column files replaces the current value).
           - If the added column files do not change the row, the existing 
`_last_updated_sequence_number` value, with null resolved as in [Row lineage 
assignment](#row-lineage-assignment), must be copied to the new column file.
   ```



##########
format/spec.md:
##########
@@ -2275,6 +2316,10 @@ Constraints are added in v4:
 * Snapshots may contain `constraint-statuses`
 * Upgrading a v2 or v3 table to v4 rewrites the table's `identifier-field-ids` 
as a `primary-key` constraint that is not enforced, unless a field cannot be 
used in a `primary-key` constraint
 
+Column updates are added in v4.

Review Comment:
   nit: column updates -> column files
   
   > Column files support is added in v4:



##########
format/spec.md:
##########
@@ -1653,6 +1656,53 @@ 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
+
+Starting in v4, some fields of a data file can be stored in separate column 
files, listed in the data file's [`column_files`](#column-file-fields), in 
addition to the data file itself. This section describes column files for data 
files.
+
+In this section:
+
+- The **previous entry** is the live entry of the data file before column 
files are added.
+- An **updated field** is a field whose ID is in the `field_ids` of an added 
column file.
+- An **updated row** is a row whose updated-field values were set by the 
update. Other rows keep their current values.
+
+A column file is a valid Iceberg data file: files must use valid Iceberg 
formats, schemas, and column projection.
+
+A column file must have the same row count as its data file.
+
+A column file stores the entire value of each field whose ID is in its 
`field_ids`. Each ID in `field_ids` must be the ID of a top-level field in the 
table schema used to write the column file, or the ID of a row lineage field 
required by [Adding Column Files](#adding-column-files). The `field_ids` of a 
column file may overlap with the field IDs of the data file. Which file a field 
is read from is determined by [Reading Column Files](#reading-column-files).
+
+#### Reading Column Files
+
+A projected field whose top-level field ID is in a column file's `field_ids`, 
including `_last_updated_sequence_number`, must be read from that column file. 
All other projected fields are read from the data file using [Column 
Projection](#column-projection).
+
+Row `i` of a column file corresponds to row `i` of the data file. Column files 
and the data file are combined by row position before the deletion vector is 
applied. Readers must ignore top-level fields in a column file whose IDs are 
not in its `field_ids`. A null value in a column file does not indicate that a 
row is deleted.
+
+#### Adding Column Files
+
+When column files are added to a data file, 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.
+
+A new column file stores the value of each of its listed fields for every row 
of the data file, including rows the update did not change. Values in rows 
deleted by the MODIFIED entry's `deletion_vector` should be null, so a 
top-level field must be optional in the column file's schema if the column file 
stores null for it. For a top-level field that is required in the table schema, 
a non-null value must be stored in every row that is not deleted by the 
MODIFIED entry's `deletion_vector`.
+
+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 other fields of the MODIFIED 
entry must match the previous entry, except for the following:
+
+- `spec_id` and `partition`:
+    - If an updated field is, or contains, a source field of the partition 
spec, `spec_id` and `partition` must be null.
+- `sort_order_id`:
+    - If an updated field is, or contains, a field in the sort order, 
`sort_order_id` must be null.
+- `content_stats`:
+    - Stats for each updated field, including its nested fields, must be 
computed from the values in the column file that has its ID in `field_ids`, or 
omitted. Stats for other fields may be kept.
+- `deletion_vector`:

Review Comment:
   I am still seeing the deletion_vector bullet.



##########
format/spec.md:
##########
@@ -1748,7 +1751,7 @@ The rows in the delete file must be sorted by `file_path` 
then `pos` to optimize
 
 Equality delete files identify deleted rows in a collection of data files by 
one or more column values, and may optionally contain additional columns of the 
deleted row.
 
-Equality delete files must not be added to v4 tables. Equality deletes cannot 
be added as an entry to a v4 manifest. Equality delete files referenced by v2 
or v3 delete manifests must still be applied in a v4 table.
+Equality delete files must not be added to v4 tables. Equality deletes cannot 
be added as an entry to a v4 manifest. Equality delete files referenced by v2 
or v3 delete manifests must still be applied in a v4 table. Equality delete 
files cannot apply to a data file that has column files (see [Writing Column 
Files](#writing-column-files)).

Review Comment:
   Should we also explain the reason why equality deletes must be rewritten 
before attaching column files?
   
   ```suggestion
   Equality delete files must not be added to v4 tables. Equality deletes 
cannot be added as an entry to a v4 manifest. Equality delete files referenced 
by v2 or v3 delete manifests must still be applied in a v4 table. 
   
   Adding column files sets the data file's data sequence number to the 
sequence number of that snapshot, so equality delete files no longer apply to 
it. Writers must first rewrite equality deletes into the data file's 
`deletion_vector` (see [Writing Column Files](#writing-column-files)) before 
attaching column files.
   ```



##########
format/spec.md:
##########
@@ -1801,6 +1804,44 @@ 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
+
+Starting in v4, a data file or v4 leaf manifest may store fields in column 
files listed in [`column_files`](#column-file-fields). A column file can use 
any supported Iceberg file format and must have rows that align with the file 
it modifies.
+
+A column file stores the fields listed in its `field_ids`, each as a whole, 
including all of its nested fields. Each ID must identify a field in the schema 
of the file it modifies, which is the table schema for a data file and the 
tracked file schema for a leaf manifest, or a row lineage field required by 
[Writing Column Files](#writing-column-files). Fields nested within a list or 
map must not be listed in `field_ids`.
+
+#### Writing Column Files
+
+The rules below apply to column files of data files. Rules for writing column 
files of leaf manifests are not defined here.
+
+A column file stores the value of each of its listed fields for every row of 
the data file. Values that did not change are still included, and values for 
deleted rows should be null.
+
+When column files are added to a data file, 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 other fields of the MODIFIED 
entry must match the previous entry, except for the following:
+
+- `partition`:
+    - If an added column file lists a field that is, or contains, a source 
field of the data file's partition spec, every row that is not deleted must 
have the same partition values after the update, and `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 computed from that column file, or omitted.
+- `tight_stats`:
+    - `tight_stats` must be false or omitted.

Review Comment:
   is this correct? writer can produce column files while still maintain the 
tight_stats property for modified columns



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