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


##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.
+
+A column file stores the entire value of each field in its `field_ids`. Each 
ID must identify a field in the table schema, or a row lineage field required 
by [Writing Column Files](#writing-column-files). A schema field must be 
top-level or nested in structs, must not be nested within a list or map, and 
must not be, or contain, a source field of the data file's partition spec.
+
+#### Writing Column Files
+
+In this subsection, the **previous entry** is the live entry of the data file 
before column files are added, an **updated field** is a table schema field 
whose ID is in the `field_ids` of an added column file, and an **updated row** 
is a row whose updated-field values were set by the update. Other rows keep 
their current values.
+
+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 for deleted 
rows should be null, so every field in `field_ids` must be optional in the 
column file's schema.

Review Comment:
   I took your suggestion and removed the nullability here. 



##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.
+
+A column file stores the entire value of each field in its `field_ids`. Each 
ID must identify a field in the table schema, or a row lineage field required 
by [Writing Column Files](#writing-column-files). A schema field must be 
top-level or nested in structs, must not be nested within a list or map, and 
must not be, or contain, a source field of the data file's partition spec.

Review Comment:
   I looked at the notes from our syncs. We discussed setting this to null. A 
null tuple means unpartitioned in the tracked file, which @stevenzwu pointed 
out, and it would also lose identity partition values on Hive tables migrated 
by metadata only, because those files don't store the partition columns.
   
   @gaborkaszab if the new values span more than one partition, the update 
isn't allowed and the writer has to rewrite those rows instead. I'd rather not 
set the entry to unpartitioned.
   
   We also later discussed just the file having the right partition spec. So 
went with "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, a
   nd `partition` must be set to those values."
   
   Let me know what you think?



##########
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 deletes 
that apply to a data file must be written into its deletion vector when column 
files are added to it (see [Writing Column Files](#writing-column-files)).

Review Comment:
   Done.



##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.
+
+A column file stores the entire value of each field in its `field_ids`. Each 
ID must identify a field in the table schema, or a row lineage field required 
by [Writing Column Files](#writing-column-files). A schema field must be 
top-level or nested in structs, must not be nested within a list or map, and 
must not be, or contain, a source field of the data file's partition spec.
+
+#### Writing Column Files
+
+In this subsection, the **previous entry** is the live entry of the data file 
before column files are added, an **updated field** is a table schema field 
whose ID is in the `field_ids` of an added column file, and an **updated row** 
is a row whose updated-field values were set by the update. Other rows keep 
their current values.

Review Comment:
   Okay, yeah @stevenzwu also suggested dropping it so I will drop this line



##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.
+
+A column file stores the entire value of each field in its `field_ids`. Each 
ID must identify a field in the table schema, or a row lineage field required 
by [Writing Column Files](#writing-column-files). A schema field must be 
top-level or nested in structs, must not be nested within a list or map, and 
must not be, or contain, a source field of the data file's partition spec.
+
+#### Writing Column Files
+
+In this subsection, the **previous entry** is the live entry of the data file 
before column files are added, an **updated field** is a table schema field 
whose ID is in the `field_ids` of an added column file, and an **updated row** 
is a row whose updated-field values were set by the update. Other rows keep 
their current values.
+
+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 for deleted 
rows should be null, so every field in `field_ids` must be optional in the 
column file's schema.
+
+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.
+
+Adding column files raises the data file's data sequence number, so deletes in 
delete manifests would stop applying to it. The MODIFIED entry's 
`deletion_vector` must therefore contain all deletes that apply to the data 
file, from equality delete files, v2 position delete files and deletion vectors 
in delete 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 `deletion_vector` and the 
following:
+
+- `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 an updated field and its nested fields must be computed from 
the column file that lists it, or omitted.
+- `tight_stats`:
+    - `tight_stats` must be false or omitted.
+- `column_files`:
+    - Each updated field ID 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 new column file must list `_last_updated_sequence_number` in its 
`field_ids`, with a null value for updated rows and the row's current non-null 
value for other rows that are not deleted.
+
+#### Reading Column Files
+
+A projected field whose 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).
+
+Column files and the data file are aligned by row position before the deletion 
vector is applied.
+

Review Comment:
   Added a line about schema here. 



##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.
+
+A column file stores the entire value of each field in its `field_ids`. Each 
ID must identify a field in the table schema, or a row lineage field required 
by [Writing Column Files](#writing-column-files). A schema field must be 
top-level or nested in structs, must not be nested within a list or map, and 
must not be, or contain, a source field of the data file's partition spec.
+
+#### Writing Column Files
+
+In this subsection, the **previous entry** is the live entry of the data file 
before column files are added, an **updated field** is a table schema field 
whose ID is in the `field_ids` of an added column file, and an **updated row** 
is a row whose updated-field values were set by the update. Other rows keep 
their current values.
+
+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 for deleted 
rows should be null, so every field in `field_ids` must be optional in the 
column file's schema.
+
+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.
+
+Adding column files raises the data file's data sequence number, so deletes in 
delete manifests would stop applying to it. The MODIFIED entry's 
`deletion_vector` must therefore contain all deletes that apply to the data 
file, from equality delete files, v2 position delete files and deletion vectors 
in delete 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 `deletion_vector` and the 
following:
+
+- `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 an updated field and its nested fields must be computed from 
the column file that lists it, or omitted.
+- `tight_stats`:
+    - `tight_stats` must be false or omitted.
+- `column_files`:
+    - Each updated field ID 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 new column file must list `_last_updated_sequence_number` in its 
`field_ids`, with a null value for updated rows and the row's current non-null 
value for other rows that are not deleted.
+
+#### Reading Column Files
+
+A projected field whose 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).

Review Comment:
   Agree with both of you. On the read side we can skip naming the special 
columns.



##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.

Review Comment:
   Sure. Done.



##########
format/spec.md:
##########
@@ -1801,6 +1804,40 @@ 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 may store fields in column files listed in 
[`column_files`](#column-file-fields). A column file is an Iceberg data file 
and must have the same row count as its data file.
+
+A column file stores the entire value of each field in its `field_ids`. Each 
ID must identify a field in the table schema, or a row lineage field required 
by [Writing Column Files](#writing-column-files). A schema field must be 
top-level or nested in structs, must not be nested within a list or map, and 
must not be, or contain, a source field of the data file's partition spec.
+
+#### Writing Column Files
+
+In this subsection, the **previous entry** is the live entry of the data file 
before column files are added, an **updated field** is a table schema field 
whose ID is in the `field_ids` of an added column file, and an **updated row** 
is a row whose updated-field values were set by the update. Other rows keep 
their current values.
+
+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 for deleted 
rows should be null, so every field in `field_ids` must be optional in the 
column file's schema.
+
+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.
+
+Adding column files raises the data file's data sequence number, so deletes in 
delete manifests would stop applying to it. The MODIFIED entry's 
`deletion_vector` must therefore contain all deletes that apply to the data 
file, from equality delete files, v2 position delete files and deletion vectors 
in delete manifests.

Review Comment:
   I moved it as you suggested. Agreed that we probably don't need to explain 
why in the spec.



##########
format/spec.md:
##########
@@ -2275,6 +2312,11 @@ 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.
+
+* Data files may have column files that store the values of one or more fields 
(see [Column Files](#column-files))
+* Adding column files to a data file raises its data sequence number to that 
of the snapshot in which they were added

Review Comment:
   Removed.



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