lliangyu-lin commented on code in PR #12821:
URL: https://github.com/apache/iceberg/pull/12821#discussion_r2049327924


##########
core/src/main/java/org/apache/iceberg/view/ViewMetadata.java:
##########
@@ -249,9 +249,23 @@ public Builder setCurrentVersionId(int newVersionId) {
         changes.add(new MetadataUpdate.SetCurrentViewVersion(newVersionId));
       }
 
+      // Use the timestamp from the view version if it was added in current 
set of changes;

Review Comment:
   @nastra Thanks for the review! I'll apply the comments and raise a revision.
   But some concerns I have right now is that the change breaks existing unit 
tests like
   
https://github.com/apache/iceberg/blob/9fc49e187069c7ec2493ac0abf20f73175b3df89/core/src/test/java/org/apache/iceberg/view/TestViewMetadataParser.java#L218-L239
   In `.setCurrentVersionId(2)`, it's in different builder and have no local 
`changes`, causing the activation of view version to set to current timestamp 
even though version 2 never set as current before. And in the comment, we are 
trying to say only `view version was set as current in the past` with use 
current ts.
   



-- 
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-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to