Repository navigation
Conversation
Adds a v4-only row lineage 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 timestamp_ms field that is null for newly added entries and inherited from the snapshot timestamp-ms, then persisted when entries are carried over. Generated-by: Cursor
…mestamp Renames the row lineage column to _last_updated_time and the tracking field to snapshot_timestamp, both timestamptz, so the commit time is directly usable in SQL predicates and metadata tables. The snapshot timestamp-ms stays a long in milliseconds and is converted to microseconds when inherited. Generated-by: Cursor
|
Following Micah's suggestion, The snapshot's |
|
|
||
| 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. |
There was a problem hiding this comment.
do we want to allow for backfilling of if snapshot information is still around in metadata.json.
There was a problem hiding this 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.
There was a problem hiding this comment.
with the rewrite of this upgrade behavior paragraph (see the other thread), my above reply about null _last_updated_time is obsolete now.
Tightens the wording of the commit timestamp inheritance rules: states that snapshot_timestamp holds timestamp-ms converted to microseconds, drops redundant rationale from the column-file bullet, says that other carried-over entries keep their previously persisted value, and requires rows from data files added or updated before the v4 upgrade to have a null _last_updated_time. Generated-by: Cursor
c1ba82c to
37e959f
Compare
| 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 |
There was a problem hiding this 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).
There was a problem hiding this 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.
There was a problem hiding this 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.
There was a problem hiding this comment.
Based on today's discussion, I updated the spec PR regarding upgrade behavior.
emkornfield
left a comment
There was a problem hiding this comment.
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 |
Root manifest entries for pre-v4 manifests are written with a null snapshot_timestamp in the upgrade snapshot and inherit its timestamp-ms, and entries in those manifests inherit from the root entry. Rows added before the upgrade get the upgrade commit time as _last_updated_time, an upper bound of when they were last updated, which survives rewrites and deletion vector updates. With every carried-over entry persisting a non-null value, readers inherit any null snapshot_timestamp regardless of entry status. Generated-by: Cursor
yes compression, or more specifically encoding. Delta binary packed would likely do better with milliseconds. Take for example millisecond values: vs microseconds: |
|
Thanks @emkornfield, you're right. I ran a quick Parquet benchmark (40K rows): 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 overhead is negligible compare to other fields (especially |
Upgrading to v4 does not rewrite the manifest list, so manifest lists and manifests written before v4 are read with a null snapshot_timestamp and inherit the snapshot's timestamp-ms. Generated-by: Cursor
Every entry now has a non-null snapshot_timestamp after inheritance, including entries in manifests written before v4, so readers always replace a null _last_updated_time with the data file's snapshot_timestamp. Generated-by: Cursor
Co-authored-by: Russell Spitzer <russell.spitzer@GMAIL.COM>
Co-authored-by: Russell Spitzer <russell.spitzer@GMAIL.COM>
Co-authored-by: Russell Spitzer <russell.spitzer@GMAIL.COM>
Co-authored-by: Russell Spitzer <russell.spitzer@GMAIL.COM>
Use "snapshot timestamp" instead of "commit timestamp", rename the section to Snapshot Timestamp Inheritance, and simplify the read inheritance rules by entry location. Generated-by: Cursor
Co-authored-by: Huaxin Gao <huaxin.gao11@gmail.com>
Adds a v4-only row lineage
_last_updated_timemetadata 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 trackingsnapshot_timestampfield that can be null and inherited from the snapshottimestamp-ms, then persisted when entries are carried over.Generated-by: Cursor