You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "stevenzwu (via GitHub)" <gi...@apache.org> on 2026/10/15 18:22:22 UTC
[PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
stevenzwu opened a new pull request, #18526:
URL: https://github.com/apache/iceberg/pull/18526
Adds a v4-only row lineage `_last_updated_time` metadata column that records the commit timestamp of the snapshot that last updated a row. Values are inherited like `_last_updated_sequence_number`, using a new tracking `snapshot_timestamp` field that can be null and inherited from the snapshot `timestamp-ms`, then persisted when entries are carried over.
Generated-by: Cursor
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461863253
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, `snapshot_timestamp` is inherited by replacing `null` with the snapshot's `timestamp-ms` converted to microseconds for entries in the root manifest, or with the `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest for entries in a leaf manifest.
Review Comment:
I think this should just say that it is inherited from its parent (either the snapshot, root manifest, or leaf manifest).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364642128
##########
format/spec.md:
##########
@@ -1053,7 +1059,7 @@ A simple (and recommended) way for writers to adapt existing metadata for table
#### Inheritance
-Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file.
+Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. In v4, `snapshot_timestamp` is also inherited when `null`; see [Commit Timestamp Inheritance](#commit-timestamp-inheritance).
Review Comment:
Commit -> Snapshot
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461858630
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
Review Comment:
Combine with the first row since this is a single statement.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288610155
##########
format/spec.md:
##########
@@ -1071,6 +1077,25 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have its inherited or previously persisted `snapshot_timestamp`.
+
+When reading, a `null` commit timestamp of an ADDED or MODIFIED entry is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
Review Comment:
```suggestion
* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest (after inheritance rules are applied to the root manifest entry).
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364566776
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, a `null` commit timestamp is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
+
+Manifest lists and manifests written before a table was upgraded to v4 do not have a `snapshot_timestamp` field and are read as if it is `null`. When reading a manifest list written before v4, each manifest inherits the snapshot's `timestamp-ms` converted to microseconds, and entries in a manifest written before v4 inherit the commit timestamp of their manifest. As a result, rows in the table at the time of the upgrade have a `_last_updated_time` equal to the commit time of the last snapshot before the upgrade.
Review Comment:
```suggestion
Manifest lists and manifests written before a table was upgraded to v4 do not have a `snapshot_timestamp` field and the field should be read as if it is `null`. When reading a manifest list written before v4, each manifest inherits the snapshot's `timestamp-ms` converted to microseconds, and entries in a manifest written before v4 inherit the commit timestamp of their manifest. As a result, rows in the table at the time of the upgrade have a `_last_updated_time` equal to the commit time of the last snapshot before the upgrade.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364632520
##########
format/spec.md:
##########
@@ -483,13 +485,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Commit Timestamp Inheritance](#commit-timestamp-inheritance).
Review Comment:
I'd switch this to Snapshot Timestamp Inheritance
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "zhjwpku (via GitHub) via issues" <is...@iceberg.apache.org>.
zhjwpku commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4429895603
##########
format/spec.md:
##########
@@ -1053,7 +1059,7 @@ A simple (and recommended) way for writers to adapt existing metadata for table
#### Inheritance
-Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file.
+Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. In v4, `snapshot_timestamp` is also inherited when `null`; see [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
I noticed that in other places it's `In v4 and later` but here it's just `In v4`, would be better to keep it consistent, but not a blocking one.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461760751
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
I think that `timestamptz` makes more sense than `timestamp`. Timestamp with time zone represents a point in time that will be shown to users in the SQL session time zone. If we were to use timestamp without time zone, the only difference would be that the time would be shown to all users the same way. In either case, the value is stored as some offset from the unix epoch. I think showing the value in the session time zone is what users expect so we should use `timestamptz`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461766013
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
I have a slight preference for `_last_updated_timestamp` to indicate that this is not just a time, but this is minor.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364666549
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, a `null` commit timestamp is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
+
+Manifest lists and manifests written before a table was upgraded to v4 do not have a `snapshot_timestamp` field and the field should be read as if it is `null`. When reading a manifest list written before v4, each manifest inherits the snapshot's `timestamp-ms` converted to microseconds, and entries in a manifest written before v4 inherit the commit timestamp of their manifest. As a result, rows in the table at the time of the upgrade have a `_last_updated_time` equal to the commit time of the last snapshot before the upgrade.
Review Comment:
```suggestion
Manifest lists and manifests written before a table was upgraded to v4 do not have a `snapshot_timestamp` field and the field should be read as if it is `null`. When reading a manifest list written before v4, each manifest inherits the snapshot's `timestamp-ms` converted to microseconds, and entries in a manifest written before v4 inherit that manifest's `snapshot_timestamp`. As a result, rows in the table at the time of the upgrade have a `_last_updated_time` equal to the `timestamp-ms` of the last snapshot before the upgrade, converted to microseconds.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364652250
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
Review Comment:
Dropping the parenthetical and "commit-time"
```suggestion
In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364562107
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, a `null` commit timestamp is inherited:
Review Comment:
I find these lines a bit hard to read. There are a lot of clauses here.
maybe
Commit timestamps are inherited differently depending on the entry location:
* Root Manifest entries inherit from the snapshot's `timestamp-ms` converted to microseconds
* Leaf Manifest entries inherit from commit timestamp of the Root Manifest
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#issuecomment-6209407064
Thanks @emkornfield, you're right. I ran a quick Parquet benchmark (40K rows): [gist.github.com/stevenzwu/401035e47dd8acb355fb9a0726789e59](https://gist.github.com/stevenzwu/401035e47dd8acb355fb9a0726789e59)
Microseconds derived from milliseconds carry three trailing zeros that the encodings don't exploit (exactly +10 bits per value with delta binary packed). With Iceberg's defaults (dictionary/plain + zstd), timestamptz µs is 1.65–2.1x larger when every row is distinct, and 1.3–1.7x when every 10 rows share a value.
In practice the impact should be small. New and modified rows are written as null and inherit, so values are only stored for rows carried over by rewrites, and those are highly repetitive (rows from the same commit share a timestamp), around 0.1–0.3 bytes per row. tracking.snapshot_timestamp is stored once per file in manifests, so its overhead is negligible compare to other fields (especially `content_stats`) for the TrackedFile schema for v4 manifest file.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4291529571
##########
format/spec.md:
##########
@@ -1071,6 +1077,25 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have its inherited or previously persisted `snapshot_timestamp`.
+
+When reading, a `null` commit timestamp of an ADDED or MODIFIED entry is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
+
+For an entry with any other status, a `null` commit timestamp is not inherited and indicates that the commit timestamp is unknown.
+
+Files and manifests added before a table was upgraded to v4 have an unknown commit timestamp. Manifests written before v4 do not have a `snapshot_timestamp` field and their entries must be read with a `null` commit timestamp. When writing an entry that was added before the upgrade to a new manifest, writers must write a `null` `snapshot_timestamp`. If such an entry is MODIFIED because its deletion vector changed, its `null` commit timestamp is inherited when reading.
Review Comment:
with the rewrite of this upgrade behavior paragraph (see the other thread), my above reply about `null` `_last_updated_time` is obsolete now.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288820068
##########
format/spec.md:
##########
@@ -2110,6 +2135,18 @@ Equality deletes are prohibited in v4.
* Upgrading a v2 or v3 table to v4 does not require rewriting data or delete files
* Readers must continue to apply equality deletes for v2 and v3 tables and for equality deletes carried over into upgraded v4 tables
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an ADDED or MODIFIED entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
A weird edge-case we might want to be explicit about:
Is if you rewrite a data file that was added prior to V4 it ends up with a null timestamp. If you then rewrite a second time, I think you end up populating the the timestamp field with the second rewrite because you can no longer determine the change?
The alternative might would be an explicit sentinel value for unknown cases? (either something like year 1 or snapshot time at conversion).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364626160
##########
format/spec.md:
##########
@@ -463,15 +463,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The commit time of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
#### Row Lineage
In v3 and later, an Iceberg table must track row lineage fields for all newly created rows. Engines must maintain the `next-row-id` table field and the following row-level fields when writing data files:
* `_row_id` a unique long identifier for every row within the table. The value is assigned via inheritance when a row is first added to the table.
* `_last_updated_sequence_number` the sequence number of the commit that last updated a row. The value is inherited when a row is first added or modified.
+* `_last_updated_time` (v4 and later) the commit time of the snapshot that last updated a row. The value is inherited when a row is first added or modified.
-These fields are assigned and updated by inheritance because the commit sequence number and starting row ID are not assigned until the snapshot is successfully committed. Inheritance is used to allow writing data and manifest files before values are known so that it is not necessary to rewrite data and manifest files when an optimistic commit is retried.
+These fields are assigned and updated by inheritance because the commit sequence number, commit timestamp, and starting row ID are not assigned until the snapshot is successfully committed. Inheritance is used to allow writing data and manifest files before values are known so that it is not necessary to rewrite data and manifest files when an optimistic commit is retried.
Review Comment:
I think we should attempt to remove "commit timestamp" from the vocabulary and stick to "snapshot-timestamp" and "_last_updated_time"
So for example here we would say "snapshot timestamp"
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461800390
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-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 snapshot root file and is documented in [Snapshot Root File First Row ID Assignment](#snapshot-root-file-first-row-id-assignment). Starting 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).
-When an existing row is moved to a different data file for any reason, writers should write `_row_id` and `_last_updated_sequence_number` according to the following rules:
+When an existing row is moved to a different data file for any reason, writers should write `_row_id`, `_last_updated_sequence_number`, and in v4 and later `_last_updated_time` according to the following rules:
1. The row's existing non-null `_row_id` must be copied into the new data file
-2. If the write has modified the row, the `_last_updated_sequence_number` field must be set to `null` (so that the modification's sequence number replaces the current value)
-3. If the write has not modified the row, the existing non-null `_last_updated_sequence_number` value must be copied to the new data file
+2. If the write has modified the row, the `_last_updated_sequence_number` field and, in v4 and later, the `_last_updated_time` field must be set to `null` (so that the modification's sequence number and snapshot timestamp replace the current values)
+3. If the write has not modified the row, the existing non-null `_last_updated_sequence_number` value and, in v4 and later, the existing non-null `_last_updated_time` value must be copied to the new data file
Review Comment:
```suggestion
3. If the write has not modified the row, the existing non-null `_last_updated_sequence_number` and `_last_updated_time` values must be copied to the new data file
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "amogh-jahagirdar (via GitHub) via issues" <is...@iceberg.apache.org>.
amogh-jahagirdar commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461805282
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
Yeah big +1 to using timestamptz, I think we don't want any ambiguity in how to interpret the timezone across different clients
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461780775
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
Either way, this is going to get the millis-from-epoch value stored in the snapshot's `timestamp-ms` field (translated to micros).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461828165
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
Review Comment:
```suggestion
The `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461872761
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, `snapshot_timestamp` is inherited by replacing `null` with the snapshot's `timestamp-ms` converted to microseconds for entries in the root manifest, or with the `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest for entries in a leaf manifest.
+
+Manifest lists and manifests written before a table was upgraded to v4 do not have a `snapshot_timestamp` field and the field should be read as if it is `null`. When reading a manifest list written before v4, each manifest inherits the snapshot's `timestamp-ms` converted to microseconds, and entries in a manifest written before v4 inherit that manifest's `snapshot_timestamp`. As a result, rows in the table at the time of the upgrade have a `_last_updated_time` equal to the `timestamp-ms` of the last snapshot before the upgrade, converted to microseconds.
Review Comment:
I think this should be in the section on upgrading, "Row Lineage for Upgraded Tables".
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288883521
##########
format/spec.md:
##########
@@ -2110,6 +2135,18 @@ Equality deletes are prohibited in v4.
* Upgrading a v2 or v3 table to v4 does not require rewriting data or delete files
* Readers must continue to apply equality deletes for v2 and v3 tables and for equality deletes carried over into upgraded v4 tables
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an ADDED or MODIFIED entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
> if you rewrite a data file that was added prior to V4 it ends up with a null timestamp.
this would generates a new entry in pre-v4 manifest file that has a different manifest schema and file format and doesn't have the `snapshot_timestamp` field.
> If you then rewrite a second time, I think you end up populating the the timestamp field with the second rewrite because you can no longer determine the change?
for v4 tables, the writer is required to persist the carried-over `_last_update_time` values for non-modified rows and `null` for modified rows. But in this case, both non-modified and modified rows would have `null` values. Inheritance kick in for both categories, which would produce wrong timestamp values for non-modified rows. Please let me know if I understand it correctly.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4289164839
##########
format/spec.md:
##########
@@ -2110,6 +2135,18 @@ Equality deletes are prohibited in v4.
* Upgrading a v2 or v3 table to v4 does not require rewriting data or delete files
* Readers must continue to apply equality deletes for v2 and v3 tables and for equality deletes carried over into upgraded v4 tables
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an ADDED or MODIFIED entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
Thanks @emkornfield. I updated the PR to go with your "snapshot time at conversion" option. v4 upgrade requires a new snapshot that rewrites the manifest list file to a root manifest file. I thought it is reasonable sentinel value.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288820415
##########
format/spec.md:
##########
@@ -1071,6 +1077,25 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have its inherited or previously persisted `snapshot_timestamp`.
+
+When reading, a `null` commit timestamp of an ADDED or MODIFIED entry is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
+
+For an entry with any other status, a `null` commit timestamp is not inherited and indicates that the commit timestamp is unknown.
+
+Files and manifests added before a table was upgraded to v4 have an unknown commit timestamp. Manifests written before v4 do not have a `snapshot_timestamp` field and their entries must be read with a `null` commit timestamp. When writing an entry that was added before the upgrade to a new manifest, writers must write a `null` `snapshot_timestamp`. If such an entry is MODIFIED because its deletion vector changed, its `null` commit timestamp is inherited when reading.
Review Comment:
looks like there was race condition. I also thought the old paragraph is problematic on describing pre-v4 manifest files. the new paragraph stayed away from it and only describe that rows from data files added/updated before v4 upgrade must have `null` `_last_updated_time`. please check if the new wording reads better.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4383716347
##########
format/spec.md:
##########
@@ -799,6 +803,7 @@ The contents of a file are part of a snapshot if its tracking `status` is **live
| _optional_ | 5 | **`modified_snapshot_id`** | `long` | Snapshot ID where the file was last modified. |
| _optional_ | 3 | **`sequence_number`** | `long` | Data sequence number of the file. Inherited when null. See [Sequence Number Inheritance](#sequence-number-inheritance). |
| _optional_ | 4 | **`file_sequence_number`** | `long` | File sequence number indicating when the file was added. Inherited when null. See [Sequence Number Inheritance](#sequence-number-inheritance). |
+| _optional_ | 8 | **`snapshot_timestamp`** | `timestamptz` | `timestamp-ms` of the snapshot that added or last modified the file's data, in microseconds. Inherited when null. See [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance). |
Review Comment:
This row is missing the 4 leading spaces that the other rows in this table have, so it will break the table rendering (the table is inside a list). The suggestion also aligns the columns with the other rows.
```suggestion
| _optional_ | 8 | **`snapshot_timestamp`** | `timestamptz` | `timestamp-ms` of the snapshot that added or last modified the file's data, in microseconds. Inherited when null. See [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance). |
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "danielcweeks (via GitHub) via issues" <is...@iceberg.apache.org>.
danielcweeks commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461632728
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
Per my above comment, the snapshot timestamp is without timezone, so timestamptz cannot be inherited.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461784544
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
#### Row Lineage
In v3 and later, an Iceberg table must track row lineage fields for all newly created rows. Engines must maintain the `next-row-id` table field and the following row-level fields when writing data files:
* `_row_id` a unique long identifier for every row within the table. The value is assigned via inheritance when a row is first added to the table.
* `_last_updated_sequence_number` the sequence number of the commit that last updated a row. The value is inherited when a row is first added or modified.
+* `_last_updated_time` (v4 and later) the timestamp of the snapshot that last updated a row. The value is inherited when a row is first added or modified.
Review Comment:
I think this is the only place where v4 and later should be used. It is distracting in the other cases.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461839541
##########
format/spec.md:
##########
@@ -2300,6 +2320,15 @@ 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
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
These bullet points are too low-level. This section doesn't need to restate the spec, it only needs to note that `snapshot_timestamp` and `_last_updated_time` were added.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_timestamp metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub) via issues" <is...@iceberg.apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4463420548
##########
format/spec.md:
##########
@@ -1093,6 +1098,7 @@ IDs in the range `10_000` (inclusive) to `200_000_000` (exclusive) are reserved
| Reserved field | ID | `base-id` | Range end |
|---------------------------------|------------|-----------|-----------|
+| `_last_updated_time` | 2147483538 | 8800 | 8999 |
Review Comment:
I wasn't sure if this should grow downwards or backwards earlier. Agree with you that we should fill out the 9000 space first.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#issuecomment-6203541379
> it might be nice to simulate how much extra storage microseconds take, to way if the UX benefit is worth the storage cost (ideally we could improve parquet to encode this better so it might be a miner concern). Left some other comments that might be worth discussing as well.
both are stored as a `long` field. why would there be any storage size difference? are you thinking about compression?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288821423
##########
format/spec.md:
##########
@@ -2110,6 +2135,18 @@ Equality deletes are prohibited in v4.
* Upgrading a v2 or v3 table to v4 does not require rewriting data or delete files
* Readers must continue to apply equality deletes for v2 and v3 tables and for equality deletes carried over into upgraded v4 tables
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an ADDED or MODIFIED entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
A weird edge-case we might want to be explicit about:
Is if you rewrite a data file that was added prior to V4 it ends up with a null timestamp. If you then rewrite a second time, I think you end up populating the the timestamp field with the second rewrite because you can no longer determine the change?
The alternative might would be an explicit sentinel value for unknown cases? (either something like year 1 or snapshot time at conversion).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364654877
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, a `null` commit timestamp is inherited:
Review Comment:
Also commit -> snapshot
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4330708606
##########
format/spec.md:
##########
@@ -2110,6 +2135,18 @@ Equality deletes are prohibited in v4.
* Upgrading a v2 or v3 table to v4 does not require rewriting data or delete files
* Readers must continue to apply equality deletes for v2 and v3 tables and for equality deletes carried over into upgraded v4 tables
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an ADDED or MODIFIED entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
Based on today's discussion, I updated the spec PR regarding upgrade behavior.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "danielcweeks (via GitHub) via issues" <is...@iceberg.apache.org>.
danielcweeks commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461599889
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
Why is this `timestamptz` vs `timestamp`? Other fields we tract timestamp values as long would be equivalent to timestamp without timezone. This seems strange that we would use a timezone specific representation.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "danielcweeks (via GitHub) via issues" <is...@iceberg.apache.org>.
danielcweeks commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461599889
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
Why is this `timestamptz` vs `timestamp`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461795441
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-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 snapshot root file and is documented in [Snapshot Root File First Row ID Assignment](#snapshot-root-file-first-row-id-assignment). Starting 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).
-When an existing row is moved to a different data file for any reason, writers should write `_row_id` and `_last_updated_sequence_number` according to the following rules:
+When an existing row is moved to a different data file for any reason, writers should write `_row_id`, `_last_updated_sequence_number`, and in v4 and later `_last_updated_time` according to the following rules:
1. The row's existing non-null `_row_id` must be copied into the new data file
-2. If the write has modified the row, the `_last_updated_sequence_number` field must be set to `null` (so that the modification's sequence number replaces the current value)
-3. If the write has not modified the row, the existing non-null `_last_updated_sequence_number` value must be copied to the new data file
+2. If the write has modified the row, the `_last_updated_sequence_number` field and, in v4 and later, the `_last_updated_time` field must be set to `null` (so that the modification's sequence number and snapshot timestamp replace the current values)
Review Comment:
```suggestion
2. If the write has modified the row, the `_last_updated_sequence_number` and `_last_updated_time` fields must be set to `null` (so that the modification's sequence number and snapshot timestamp replace the current values)
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461839541
##########
format/spec.md:
##########
@@ -2300,6 +2320,15 @@ 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
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
This is too low-level. This section doesn't need to restate the spec, it only needs to note that `snapshot_timestamp` and `_last_updated_time` were added.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461820926
##########
format/spec.md:
##########
@@ -1219,7 +1225,7 @@ A simple (and recommended) way for writers to adapt existing metadata for table
#### Inheritance
-Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file.
+Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. In v4 and later, `snapshot_timestamp` is also inherited when `null`; see [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
```suggestion
Values for `snapshot_id`, `sequence_number`, `file_sequence_number`, and `snapshot_timestamp` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file.
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4289156323
##########
format/spec.md:
##########
@@ -2110,6 +2135,18 @@ Equality deletes are prohibited in v4.
* Upgrading a v2 or v3 table to v4 does not require rewriting data or delete files
* Readers must continue to apply equality deletes for v2 and v3 tables and for equality deletes carried over into upgraded v4 tables
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an ADDED or MODIFIED entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
this seems like a duplicate comment. resolving it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub)" <gi...@apache.org>.
stevenzwu commented on PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#issuecomment-6202582071
A note on the type choice for the `_last_updated_time` metadata column and the `tracking.snapshot_timestamp` field.
Earlier drafts used `long` milliseconds for both (`_last_updated_timestamp_ms` and `timestamp_ms`) to mirror the snapshot's `timestamp-ms`. Following Micah's suggestion, both now use `timestamptz` (microseconds):
- **SQL usability**: users can filter with timestamp literals and functions directly, e.g. `WHERE _last_updated_time > now() - INTERVAL 1 HOUR`, instead of working with epoch milliseconds, and values display as readable timestamps.
- **`timestamptz` rather than `timestamp`**: a commit time is an instant, not a local date-time. This is also consistent with existing metadata tables, where `snapshots.committed_at` and `history.made_current_at` are `timestamptz`.
- **Same type for the tracking field**: `tracking.snapshot_timestamp` uses the same type as the column, so inherited values are assigned to rows without conversion, and the field reads naturally in the `entries` and `files` metadata tables. The storage cost is the same as a `long`.
The snapshot's `timestamp-ms` is unchanged (`long` milliseconds). It is converted to microseconds once, when a root manifest entry inherits it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288770323
##########
format/spec.md:
##########
@@ -1071,6 +1077,25 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have its inherited or previously persisted `snapshot_timestamp`.
+
+When reading, a `null` commit timestamp of an ADDED or MODIFIED entry is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
+
+For an entry with any other status, a `null` commit timestamp is not inherited and indicates that the commit timestamp is unknown.
+
+Files and manifests added before a table was upgraded to v4 have an unknown commit timestamp. Manifests written before v4 do not have a `snapshot_timestamp` field and their entries must be read with a `null` commit timestamp. When writing an entry that was added before the upgrade to a new manifest, writers must write a `null` `snapshot_timestamp`. If such an entry is MODIFIED because its deletion vector changed, its `null` commit timestamp is inherited when reading.
Review Comment:
do we want to allow for backfilling of if snapshot information is still around in metadata.json.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "amogh-jahagirdar (via GitHub) via issues" <is...@iceberg.apache.org>.
amogh-jahagirdar commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4459712014
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
Review Comment:
Do we need the "so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. " statement? The rules below cover what to to do on write/read?
##########
format/spec.md:
##########
@@ -1053,7 +1059,7 @@ A simple (and recommended) way for writers to adapt existing metadata for table
#### Inheritance
-Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file.
+Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. In v4, `snapshot_timestamp` is also inherited when `null`; see [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
v4 and later (or v4+ honestly, we say "v4 and later" a lot)...
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, a `null` `snapshot_timestamp` is inherited differently depending on the entry location:
Review Comment:
I think we can drop the "inherited differently depending on.." and just have a combined statement (similar to what we do for other inheritance areas):
`
When reading, `snapshot_timestamp` is inherited by replacing `null` with the snapshot's `timestamp-ms` converted to microseconds for entries in the root manifest, or with the `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest for entries in a leaf manifest.
`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461853180
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
Review Comment:
This should be more generic since it applies to the root as well.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461787553
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-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 snapshot root file and is documented in [Snapshot Root File First Row ID Assignment](#snapshot-root-file-first-row-id-assignment). Starting 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).
-When an existing row is moved to a different data file for any reason, writers should write `_row_id` and `_last_updated_sequence_number` according to the following rules:
+When an existing row is moved to a different data file for any reason, writers should write `_row_id`, `_last_updated_sequence_number`, and in v4 and later `_last_updated_time` according to the following rules:
Review Comment:
We don't need to mention `in v4 and later` in every line.
```suggestion
When an existing row is moved to a different data file for any reason, writers should write `_row_id`, `_last_updated_sequence_number`, and `_last_updated_time` according to the following rules:
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461822952
##########
format/spec.md:
##########
@@ -1219,7 +1225,7 @@ A simple (and recommended) way for writers to adapt existing metadata for table
#### Inheritance
-Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file.
+Values for `snapshot_id`, `sequence_number`, and `file_sequence_number` are inherited from manifest metadata when `null`. That is, if the field is `null` for an entry, then the entry must inherit its value from the manifest file's metadata, stored in the snapshot root file. In v4 and later, `snapshot_timestamp` is also inherited when `null`; see [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
No need for a link to a sub-section. It is part of this section and the others don't have one.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461813795
##########
format/spec.md:
##########
@@ -1093,6 +1098,7 @@ IDs in the range `10_000` (inclusive) to `200_000_000` (exclusive) are reserved
| Reserved field | ID | `base-id` | Range end |
|---------------------------------|------------|-----------|-----------|
+| `_last_updated_time` | 2147483538 | 8800 | 8999 |
Review Comment:
I think this should be 9400-9599, right?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_timestamp metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub) via issues" <is...@iceberg.apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4463420417
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
#### Row Lineage
In v3 and later, an Iceberg table must track row lineage fields for all newly created rows. Engines must maintain the `next-row-id` table field and the following row-level fields when writing data files:
* `_row_id` a unique long identifier for every row within the table. The value is assigned via inheritance when a row is first added to the table.
* `_last_updated_sequence_number` the sequence number of the commit that last updated a row. The value is inherited when a row is first added or modified.
+* `_last_updated_time` (v4 and later) the timestamp of the snapshot that last updated a row. The value is inherited when a row is first added or modified.
Review Comment:
Removed "v4 and later" from the other places, including the reserved field ID table, and kept it only here.
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
This also matches the `snapshots.committed_at` and `history.made_current_at` metadata table columns, which are `timestamptz`.
##########
format/spec.md:
##########
@@ -2300,6 +2320,15 @@ 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
+Row lineage changes:
+
+* When writing a manifest, ADDED entries and entries MODIFIED by column files must be written with a null `snapshot_timestamp`
+* When an entry has a null `snapshot_timestamp`, readers must assign it the snapshot's `timestamp-ms` converted to microseconds in the root manifest, or the leaf manifest's `snapshot_timestamp` in a leaf manifest
Review Comment:
Reduced this to two bullets noting that `snapshot_timestamp` and `_last_updated_timestamp` were added.
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
Review Comment:
Changed to "When writing a manifest, writers must set `snapshot_timestamp` as follows:"
##########
format/spec.md:
##########
@@ -522,15 +522,17 @@ The set of metadata columns is:
| **`2147483541 _commit_snapshot_id`** | `long` | The snapshot ID in which the change occurred |
| **`2147483540 _row_id`** | `long` | A unique long assigned for row lineage, see [Row Lineage](#row-lineage) |
| **`2147483539 _last_updated_sequence_number`** | `long` | The sequence number which last updated this row, see [Row Lineage](#row-lineage) |
+| **`2147483538 _last_updated_time`** | `timestamptz` | The timestamp of the snapshot which last updated this row (v4 and later), see [Row Lineage](#row-lineage) |
Review Comment:
Renamed to `_last_updated_timestamp`.
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, `snapshot_timestamp` is inherited by replacing `null` with the snapshot's `timestamp-ms` converted to microseconds for entries in the root manifest, or with the `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest for entries in a leaf manifest.
+
+Manifest lists and manifests written before a table was upgraded to v4 do not have a `snapshot_timestamp` field and the field should be read as if it is `null`. When reading a manifest list written before v4, each manifest inherits the snapshot's `timestamp-ms` converted to microseconds, and entries in a manifest written before v4 inherit that manifest's `snapshot_timestamp`. As a result, rows in the table at the time of the upgrade have a `_last_updated_time` equal to the `timestamp-ms` of the last snapshot before the upgrade, converted to microseconds.
Review Comment:
Moved to "Row Lineage for Upgraded Tables".
##########
format/spec.md:
##########
@@ -1093,6 +1098,7 @@ IDs in the range `10_000` (inclusive) to `200_000_000` (exclusive) are reserved
| Reserved field | ID | `base-id` | Range end |
|---------------------------------|------------|-----------|-----------|
+| `_last_updated_time` | 2147483538 | 8800 | 8999 |
Review Comment:
I wasn't sure if this should grow downwards or backwards. agree with you that we should fill out the 9000 space first.
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
Review Comment:
Combined into a single bullet.
##########
format/spec.md:
##########
@@ -1237,6 +1243,20 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Snapshot Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the `timestamp-ms` of the snapshot that added or last modified a file's data, converted to microseconds. It is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+Writers must set `tracking.snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have a non-null `snapshot_timestamp` (inherited or previously persisted).
+
+When reading, `snapshot_timestamp` is inherited by replacing `null` with the snapshot's `timestamp-ms` converted to microseconds for entries in the root manifest, or with the `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest for entries in a leaf manifest.
Review Comment:
Updated to say a `null` value is inherited from its parent.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4288610155
##########
format/spec.md:
##########
@@ -1071,6 +1077,25 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
+
+In v4 and later, the `snapshot_timestamp` tracking field records the commit time (the snapshot's `timestamp-ms` converted to microseconds) of the snapshot that added or last modified a file's data, and is used to assign `_last_updated_time` for [row lineage](#row-lineage).
+
+The snapshot's `timestamp-ms` is not known until the snapshot is successfully committed and may change when a commit is retried, so writers set `snapshot_timestamp` to `null` when the value must come from the committing snapshot. Writers must set `snapshot_timestamp` for data file and leaf manifest entries as follows:
+
+* An ADDED entry must have a `null` `snapshot_timestamp`.
+* An entry MODIFIED by column files must have a `null` `snapshot_timestamp`.
+* Any other entry written to a new manifest must have its inherited or previously persisted `snapshot_timestamp`.
+
+When reading, a `null` commit timestamp of an ADDED or MODIFIED entry is inherited:
+
+* For an entry in the root manifest, from the snapshot's `timestamp-ms` converted to microseconds.
+* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest.
Review Comment:
```suggestion
* For an entry in a leaf manifest, from the inherited or persisted `snapshot_timestamp` of the root manifest entry that tracks the leaf manifest (after inheritance rules are applied to the root manifest entry).
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#issuecomment-6204321605
> both are stored as a long field. why would there be any storage size difference? are you thinking about compression?
yes compression, or more specifically encoding. Delta binary packed would likely do better with milliseconds. Take for example millisecond values: `[1, 2, 4]`-> `1 [1, 2]`, after frame of reference subtraction (-1) the vector part would `[0, 1]` so would require 1 bit per value.
vs
microseconds: `[1000, 2000, 4000]`->`1000 [1000, 2000]`, after frame of reference subtraction (-1000) the vector part would be `[0, 1000]` required 10 bits per value.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364649466
##########
format/spec.md:
##########
@@ -1071,6 +1077,23 @@ Inheriting sequence numbers through the metadata tree allows writing a new manif
Inheritance does not apply to v1, which does not have inherited fields like sequence number. When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0.
+##### Commit Timestamp Inheritance
Review Comment:
Commit -> Snapshot
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364635120
##########
format/spec.md:
##########
@@ -483,13 +485,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Commit Timestamp Inheritance](#commit-timestamp-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 snapshot root file and is documented in [Snapshot Root File First Row ID Assignment](#snapshot-root-file-first-row-id-assignment). Starting 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).
-When an existing row is moved to a different data file for any reason, writers should write `_row_id` and `_last_updated_sequence_number` according to the following rules:
+When an existing row is moved to a different data file for any reason, writers should write `_row_id`, `_last_updated_sequence_number`, and in v4 and later `_last_updated_time` according to the following rules:
1. The row's existing non-null `_row_id` must be copied into the new data file
-2. If the write has modified the row, the `_last_updated_sequence_number` field must be set to `null` (so that the modification's sequence number replaces the current value)
-3. If the write has not modified the row, the existing non-null `_last_updated_sequence_number` value must be copied to the new data file
+2. If the write has modified the row, the `_last_updated_sequence_number` field and, in v4 and later, the `_last_updated_time` field must be set to `null` (so that the modification's sequence number and commit timestamp replace the current values)
Review Comment:
commit -> snapshot
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "RussellSpitzer (via GitHub)" <gi...@apache.org>.
RussellSpitzer commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4364639309
##########
format/spec.md:
##########
@@ -799,6 +803,7 @@ The contents of a file are part of a snapshot if its tracking `status` is **live
| _optional_ | 5 | **`modified_snapshot_id`** | `long` | Snapshot ID where the file was last modified. |
| _optional_ | 3 | **`sequence_number`** | `long` | Data sequence number of the file. Inherited when null. See [Sequence Number Inheritance](#sequence-number-inheritance). |
| _optional_ | 4 | **`file_sequence_number`** | `long` | File sequence number indicating when the file was added. Inherited when null. See [Sequence Number Inheritance](#sequence-number-inheritance). |
+ | _optional_ | 8 | **`snapshot_timestamp`** | `timestamptz` | Commit time of the snapshot that added or last modified the file's data. Inherited when null. See [Commit Timestamp Inheritance](#commit-timestamp-inheritance). |
Review Comment:
```suggestion
| _optional_ | 8 | **`snapshot_timestamp`** | `timestamptz` | `timestamp-ms` of the snapshot that added or last modified the file's data, in microseconds. Inherited when null. See [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance). |
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461795441
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-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 snapshot root file and is documented in [Snapshot Root File First Row ID Assignment](#snapshot-root-file-first-row-id-assignment). Starting 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).
-When an existing row is moved to a different data file for any reason, writers should write `_row_id` and `_last_updated_sequence_number` according to the following rules:
+When an existing row is moved to a different data file for any reason, writers should write `_row_id`, `_last_updated_sequence_number`, and in v4 and later `_last_updated_time` according to the following rules:
1. The row's existing non-null `_row_id` must be copied into the new data file
-2. If the write has modified the row, the `_last_updated_sequence_number` field must be set to `null` (so that the modification's sequence number replaces the current value)
-3. If the write has not modified the row, the existing non-null `_last_updated_sequence_number` value must be copied to the new data file
+2. If the write has modified the row, the `_last_updated_sequence_number` field and, in v4 and later, the `_last_updated_time` field must be set to `null` (so that the modification's sequence number and snapshot timestamp replace the current values)
Review Comment:
```suggestion
2. If the write has modified the row, the `_last_updated_sequence_number` field and the `_last_updated_time` field must be set to `null` (so that the modification's sequence number and snapshot timestamp replace the current values)
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "rdblue (via GitHub) via issues" <is...@iceberg.apache.org>.
rdblue commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4461790744
##########
format/spec.md:
##########
@@ -542,13 +544,15 @@ 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).
+In v4 and later, `_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
Review Comment:
```suggestion
`_last_updated_time` is set in the same way as `_last_updated_sequence_number`. On read, if `_last_updated_time` is `null` it is assigned the `snapshot_timestamp` of the data file's tracking metadata, documented in [Snapshot Timestamp Inheritance](#snapshot-timestamp-inheritance).
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub) via issues" <is...@iceberg.apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4463390312
##########
format/spec.md:
##########
@@ -1093,6 +1098,7 @@ IDs in the range `10_000` (inclusive) to `200_000_000` (exclusive) are reserved
| Reserved field | ID | `base-id` | Range end |
|---------------------------------|------------|-----------|-----------|
+| `_last_updated_time` | 2147483538 | 8800 | 8999 |
Review Comment:
I wasn't sure if this should grow downwards or backwards. agree with you that we should fill out the 9000 space first.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spec: add _last_updated_time metadata column to row lineage in v4 [iceberg]
Posted by "stevenzwu (via GitHub) via issues" <is...@iceberg.apache.org>.
stevenzwu commented on code in PR #18526:
URL: https://github.com/apache/iceberg/pull/18526#discussion_r4463390312
##########
format/spec.md:
##########
@@ -1093,6 +1098,7 @@ IDs in the range `10_000` (inclusive) to `200_000_000` (exclusive) are reserved
| Reserved field | ID | `base-id` | Range end |
|---------------------------------|------------|-----------|-----------|
+| `_last_updated_time` | 2147483538 | 8800 | 8999 |
Review Comment:
I wasn't sure if this should grow downwards or backwards. agree with you that we should fill out the 9000 space first.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org