amogh-jahagirdar commented on code in PR #16025:
URL: https://github.com/apache/iceberg/pull/16025#discussion_r4266502846
##########
format/spec.md:
##########
@@ -61,7 +61,10 @@ The full set of changes are listed in [Appendix
E](#version-3).
Version 4 of the Iceberg spec restructures metadata for improved performance
and new capabilities:
+* Support for an [adaptive metadata tree](#manifests), enabling efficient
small commits, column updates, and columnar statistics representation
+* Data files and deletion vectors are stored in a combined entry
representation, removing the need for two phase planning
Review Comment:
@pvary The overview already has a bullet that writing new equality deletes
is no longer allowed, and position deletes were already disallowed in v3, so I
left this as is.
##########
format/spec.md:
##########
@@ -61,6 +61,8 @@ The full set of changes are listed in [Appendix
E](#version-3).
Version 4 of the Iceberg spec restructures metadata for improved performance
and new capabilities:
+* Support for an [adaptive metadata tree](#manifests), enabling efficient
small commits, column updates, and columnar statistics representation
+* Data files and deletion vectors are stored in a combined entry
representation, removing the need for two phase planning
Review Comment:
I went with "Data files and their corresponding deletion vector are stored
in a combined entry".
##########
format/spec.md:
##########
@@ -144,8 +146,11 @@ Version 4 of the Iceberg spec adds support for relative
locations in metadata, e
* **Schema** -- Names and types of fields in a table.
* **Partition spec** -- A definition of how partition values are derived from
data fields.
* **Snapshot** -- The state of a table at some point in time, including the
set of all data files.
-* **Manifest list** -- A file that lists manifest files; one per snapshot.
-* **Manifest** -- A file that lists data or delete files; a subset of a
snapshot.
+* **Snapshot root file** -- The per-snapshot file that tracks a snapshot's
manifests; a manifest list (v1-v3) or a root manifest (v4).
+* **Manifest list** -- (v1-v3 only) A file that lists manifest files; one per
snapshot.
+* **Root manifest** -- (v4+) A manifest that can reference leaf manifests,
data files, or v1-v3 manifests; one per snapshot.
Review Comment:
Done! I also added a "Leaf manifest" term, which is just a data or delete
manifest.
##########
format/spec.md:
##########
@@ -476,7 +481,7 @@ A data file with only new rows for the table may omit the
`_last_updated_sequenc
On read, if `_last_updated_sequence_number` is `null` it is assigned the
`sequence_number` of the data file's manifest entry. The data sequence number
of a data file is documented in [Sequence Number
Inheritance](#sequence-number-inheritance).
-When `null`, a row's `_row_id` field is assigned to the `first_row_id` from
its containing data file plus the row position in that data file (`_pos`). A
data file's `first_row_id` field is assigned using inheritance and is
documented in [First Row ID Inheritance](#first-row-id-inheritance). A
manifest's `first_row_id` is assigned when writing the manifest list for a
snapshot and is documented in [First Row ID
Assignment](#first-row-id-assignment). A snapshot's `first-row-id` is set to
the table's `next-row-id` and is documented in [Snapshot Row
IDs](#snapshot-row-ids).
+When `null`, a row's `_row_id` field is assigned to the `first_row_id` from
its containing data file plus the row position in that data file (`_pos`). A
data file's `first_row_id` field is assigned using inheritance and is
documented in [First Row ID Inheritance](#first-row-id-inheritance). A
manifest's `first_row_id` is assigned when writing the snapshot root file and
is documented in [First Row ID Assignment](#first-row-id-assignment). In v4, a
data file in the root manifest is assigned a `first_row_id` in the same way. A
snapshot's `first-row-id` is set to the table's `next-row-id` and is documented
in [Snapshot Row IDs](#snapshot-row-ids).
Review Comment:
I think merging the two sentences makes it a bit harder to parse. Since the
meaning is effectively the same and this wording originally came from @rdblue's
earlier suggestion, I'd prefer to keep it as is.
##########
format/spec.md:
##########
@@ -546,7 +551,7 @@ Note that:
### Partitioning
-Data files are stored in manifests with a tuple of partition values that are
used in scans to filter out files that cannot contain records that match the
scan’s filter predicate. Partition values for a data file must be the same for
all records stored in the data file. (Manifests store data files from any
partition, as long as the partition spec is the same for the data files.)
+Data files are stored in manifests with a tuple of partition values that are
used in scans to filter out files that cannot contain records that match the
scan’s filter predicate. Partition values for a data file must be the same for
all records stored in the data file. Manifests store data files from any
partition. v4 manifests may store partitions from any spec, but manifests in v3
and earlier store files for a single spec. A manifest is considered partitioned
by a spec if every entry in the manifest is partitioned by that spec.
Review Comment:
Done!
##########
format/spec.md:
##########
@@ -676,15 +693,26 @@ A manifest file must store the partition spec and other
metadata as properties i
| _optional_ | _required_ | `format-version` | Table format version
number of the manifest as a string
|
| | _required_ | `content` | Type of content files
tracked by the manifest: "data" or "deletes"
|
+=== "v4"
+ | Requirement | Key | Value |
+ |-------------|---------------------|-------|
+ | _required_ | `schema-id` | ID of the schema used to write the
manifest as a string |
+ | _required_ | `format-version` | Table format version number of the
manifest as a string |
+
#### Content file uniqueness
Within a snapshot, each content file must be referenced by at most one live
manifest entry across all manifests; otherwise, the snapshot has undefined
behavior. Writers should not produce multiple manifest entries for the same
content file in a snapshot (for example, both ADDED and DELETED entries for the
same file). Writers are not required to validate uniqueness at commit time.
-#### Manifest Entry Fields
+#### Manifest Schema
+
+In v4, manifests store `tracked_file` records that describe data or manifest
files and contain `tracking` metadata, like `status`. The relationship between
a file and its tracking metadata is inverted in v1-v3 manifests, which store
`manifest_entry` records that contain tracking information and a `data_file`
struct. The v4 manifest schema is not compatible with the v1-v3 manifest schema.
Review Comment:
Done!
##########
format/spec.md:
##########
@@ -656,15 +661,27 @@ A data or delete file is associated with a sort order by
the sort order's id wit
### Manifests
-A manifest is an immutable Avro file that lists data files or delete files,
along with each file’s partition data tuple, metrics, and tracking information.
One or more manifest files are used to store a [snapshot](#snapshots), which
tracks all of the files in a table at some point in time. Manifests are tracked
by a [manifest list](#manifest-lists) for each table snapshot.
+A table's metadata tree is composed of manifests. A manifest is an immutable
file that tracks a subset of a table metadata for a given
[snapshot](#snapshots). Leaf manifests are the lowest level of the metadata
tree and track data or delete files, along with each file's partition data,
metrics, and tracking information. The snapshot root file tracks leaf
manifests; in v4 and later the root is also a manifest and can also track data
files.
Review Comment:
Since it looks like there was some confusion here, I added a "Leaf manifest"
term (a data or delete manifest), which I think clears this up without
restructuring the paragraph.
--
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]
