stevenzwu commented on code in PR #16425:
URL: https://github.com/apache/iceberg/pull/16425#discussion_r4518709742
##########
format/spec.md:
##########
@@ -999,6 +1002,7 @@ The contents of a file are part of a snapshot if its
tracking `status` is **live
- `column_files`:
- `column_files` may only be set if the tracked file is a data file or
a v4 leaf manifest.
- A field ID may appear in `field_ids` of at most one column file in a
tracked file's `column_files`.
+ - If a column file lists a struct, no other column file in the tracked
file's `column_files` may list a field nested in that struct.
Review Comment:
can be tightened up a little.
```suggestion
- If a struct's ID is in one column file's `field_ids`, IDs of
fields nested in that struct must not be in another column file's `field_ids`.
```
##########
format/spec.md:
##########
@@ -1801,6 +1805,51 @@ 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`. A listed struct
includes all fields nested in it. In this section, a struct _contains_ a field
if the field is nested in the struct at any depth.
+
+When a column file is added, each ID in its `field_ids` must identify a field
in the schema of the file it modifies, or a row lineage field required by
[Writing Column Files](#writing-column-files). The schema of a data file is the
table schema. The schema of a leaf manifest is the tracked file schema. Fields
nested in a list or map must not be listed in `field_ids`.
+
+#### Writing Column Files
+
+A column file stores the value of each of its listed fields for every row of
the file it modifies. Values that did not change are still included, and values
for deleted rows should be null.
Review Comment:
"listed" is easy to confuse with the list type, and line 1812 already says
which fields a column file stores. What this sentence adds is that a column
file has a value for every row. Fields in a null struct should also be null, to
match what readers see:
```suggestion
A column file stores a value for every row of the file it modifies,
including rows whose values did not change. Values for deleted rows, and for
fields in a null struct, should be null.
```
##########
format/spec.md:
##########
@@ -1801,6 +1805,51 @@ 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`. A listed struct
includes all fields nested in it. In this section, a struct _contains_ a field
if the field is nested in the struct at any depth.
+
+When a column file is added, each ID in its `field_ids` must identify a field
in the schema of the file it modifies, or a row lineage field required by
[Writing Column Files](#writing-column-files). The schema of a data file is the
table schema. The schema of a leaf manifest is the tracked file schema. Fields
nested in a list or map must not be listed in `field_ids`.
+
+#### Writing Column Files
+
+A column file stores the value of each of its listed fields for every row of
the file it modifies. Values that did not change are still included, and values
for deleted rows should be null.
+
+The added column files must list each field that the update changes, or a
struct that contains that field. If an update changes whether a struct is null,
the update changes that struct.
+
+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. For a leaf manifest,
`tracking.sequence_number` must not change. The fields of the MODIFIED entry
outside `tracking` must match the previous entry, except for the following:
Review Comment:
Instead of an exception here, scope the two Sequence Number Inheritance
rules to data files:
- Line 1210: "… must not change unless a column file is added **to a data
file**."
- Line 1216: "… or MODIFIED by adding a column file **to a data file**."
Those lines are outside this PR's diff, so they can't take inline
suggestions.
Then we can remove the exception from this paragraph.
```suggestion
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:
```
--
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]
