-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Spec: Add added-rows field to Snapshot #11976
Spec: Add added-rows field to Snapshot #11976
Conversation
format/spec.md
Outdated
@@ -654,6 +656,7 @@ A snapshot consists of the following fields: | |||
| _optional_ | _required_ | _required_ | **`summary`** | A string map that summarizes the snapshot changes, including `operation` (see below) | | |||
| _optional_ | _optional_ | _optional_ | **`schema-id`** | ID of the table's current schema when the snapshot was created | | |||
| | | _optional_ | **`first-row-id`** | The first `_row_id` assigned to the first row in the first data file in the first manifest, see [Row Lineage](#row-lineage) | | |||
| | | _optional | **`added-rows`** | The number of newly added rows in this snapshot. Required if [Row Lineage](#row-lineage) is enabled | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format nit:
- missing trailing underscore from
_optional
- 5th and 6th
|
seem to be misaligned from rows above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally we don't do the vertical alignment, but it's pretty easy here so i'll fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense to me.
Merged, Thanks @sfc-gh-aixu, @amogh-jahagirdar , @dramaticlly , @danielcweeks , @stevenzwu , @Fokko for reviewing! |
Split out from #11948
Working on the row-lineage implementation made it clear that we needed a way to get information from the Snapshot object propagated into the Metadata layer. Specifically we need to know the count of all newly added rows in that Snapshot to change the
last-row-id
of the TableMetadata. While we can potentially read this from Snapshot Summary, it would be a bit odd to have a requirement on key value pair within Snapshot summary. We could also potentially re-read and calculate the number of added rows, but this would require re-opening the manifest list itself.I believe it makes more sense to formally have added-rows as an optional field within the Snapshot itself so we can make it clear in the spec this value is expected to be stored within the Snapshot metadata if row-lineage is enabled.