rdblue commented on code in PR #4071:
URL: https://github.com/apache/iceberg/pull/4071#discussion_r846880492
##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1300,20 +1346,13 @@ private void setBranchSnapshotInternal(Snapshot
snapshot, String branch) {
this.lastUpdatedMillis = isAddedSnapshot(snapshot.snapshotId()) ?
snapshot.timestampMillis() : System.currentTimeMillis();
- if (SnapshotRef.MAIN_BRANCH.equals(branch)) {
Review Comment:
Okay, thinking about this, I think that it is slightly different for the
other method. I think it should be this:
```java
if (isAddedSnapshot(snapshot.snapshotId()) {
this.lastUpdatedMillis = snapshot.timestampMillis();
}
if (SnapshotRef.MAIN_BRANCH.equals(branch)) {
this.currentSnapshotId = replacementSnapshotId;
if (lastUpdatedMillis == null) {
this.lastUpdatedMillis = System.currentTimeMillis();
}
snapshotLog.add(new SnapshotLogEntry(lastUpdatedMillis,
replacementSnapshotId));
}
```
That ensures that if a branch is updated to a newly added snapshot,
`lastUpdatedMillis` is set to match the new snapshot, just like we do now with
main. In addition, if `main` is set to an existing snapshot, that will use the
same timestamp for the commit and for snapshot history. And if either of those
isn't the case---main isn't changing and there isn't a new snapshot---then the
update time is set when the metadata is built.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
