You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/05/31 21:53:48 UTC

[GitHub] [iceberg] amogh-jahagirdar opened a new pull request, #4922: Core: Assign main branch to table's current snapshot if main could not be found when parsing. TableMetadata.Builder will allow setting -1 to main

amogh-jahagirdar opened a new pull request, #4922:
URL: https://github.com/apache/iceberg/pull/4922

   As part of https://github.com/apache/iceberg/pull/4428/files and other PRs, a common theme has been around checking if main even exists in different API implementations. In this PR, main will always be set in a table metadata's refs. If refs don't exist or the main ref does not exist for some reason when parsing, it will be set to the table's current snapshot. 
   
   This change also allows -1 to be set on different table metadata builder APIs to allow main to be created when there is no current table snapshot.
   
   I'm still mulling over the implications of this change, the benefit is that this change allows different operations to be implemented with the assumption that main exists. However, I think the real long term solution is that we produce a snapshot upon table creation and assign main at that point. Currently we only allow -1 for main, but this prevents the ability to create a branch before a snapshot is produced on main, which is a bit awkward but I think is needed until we produce a snapshot upon table creation. 
   
   @rdblue @jackye1995 let me know your thoughts! 


-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887388996


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1187,6 +1192,20 @@ public TableMetadata build() {
           previousFiles, previousFileLocation, base.lastUpdatedMillis(), properties);
       List<HistoryEntry> newSnapshotLog = updateSnapshotLog(snapshotLog, snapshotsById, currentSnapshotId, changes);
 
+      if (refs.isEmpty() && currentSnapshotId != -1) {

Review Comment:
   Will add tests for this if we deem setting main here is the right way to go



-- 
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


[GitHub] [iceberg] rdblue closed pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
rdblue closed pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state
URL: https://github.com/apache/iceberg/pull/4922


-- 
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


[GitHub] [iceberg] hililiwei commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
hililiwei commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r910566907


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -1187,6 +1192,20 @@ public TableMetadata build() {
           previousFiles, previousFileLocation, base.lastUpdatedMillis(), properties);
       List<HistoryEntry> newSnapshotLog = updateSnapshotLog(snapshotLog, snapshotsById, currentSnapshotId, changes);
 
+      if (refs.isEmpty() && currentSnapshotId != -1) {

Review Comment:
   +1 



-- 
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


[GitHub] [iceberg] rdblue commented on pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
rdblue commented on PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#issuecomment-1178359206

   Reopening this. I accidentally closed it through a different PR.


-- 
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


[GitHub] [iceberg] rdblue commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r886265531


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -120,13 +120,15 @@ static TableMetadata newTableMetadata(Schema schema,
     // break existing tables.
     MetricsConfig.fromProperties(properties).validateReferencedColumns(schema);
 
+    SnapshotRef main = SnapshotRef.branchBuilder(-1).build();

Review Comment:
   If the current snapshot is -1, there should be no main ref. In that case, there is no current state or main. We're moving away from using `-1` as a special snapshot ID.



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#issuecomment-1279873161

   This PR is no longer needed due to https://github.com/apache/iceberg/pull/5669 which already includes the change here


-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#issuecomment-1142707837

   Tests are failing, definitely related to this change. Looking into 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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887275295


##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -174,7 +174,7 @@ protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
   protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
                                              int numRetries) {
     refreshFromMetadataLocation(newLocation, shouldRetry, numRetries,
-        metadataLocation -> TableMetadataParser.read(io(), metadataLocation));
+        metadataLocation -> TableMetadata.buildFromLocation(io(), metadataLocation).build());

Review Comment:
   Don't know about this, here we parse the metadata and then again copy over the structures in the builder. But ultimately when we read, we need to go through a single point in the builder for setting main if it doesn't exist and there is a current snapshot. I don't think just setting the ref when parsing would work because ultimately it needs to be encapsulated in an metadata update even for operations which don't produce snapshots. @rdblue thoughts?



-- 
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


[GitHub] [iceberg] rdblue commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r886265832


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -529,6 +531,7 @@ public TableMetadata replaceProperties(Map<String, String> rawProperties) {
 
     return new Builder(this)
         .setProperties(updated)
+        .setRef(SnapshotRef.MAIN_BRANCH, SnapshotRef.branchBuilder(currentSnapshotId).build())

Review Comment:
   I don't think it is a good idea to overwrite the main branch anywhere. If there are no refs, but there is a current snapshot, then the builder should produce a main branch. I think that's the only place where this logic should be handled.



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887296820


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -120,13 +120,15 @@ static TableMetadata newTableMetadata(Schema schema,
     // break existing tables.
     MetricsConfig.fromProperties(properties).validateReferencedColumns(schema);
 
+    SnapshotRef main = SnapshotRef.branchBuilder(-1).build();

Review Comment:
   Good point, I was originally thinking that at least the ref can exist even if there is no table state. But it's easier to just maintain the invariant that so long as there's no table state, main won't exist.



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887275295


##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -174,7 +174,7 @@ protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
   protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
                                              int numRetries) {
     refreshFromMetadataLocation(newLocation, shouldRetry, numRetries,
-        metadataLocation -> TableMetadataParser.read(io(), metadataLocation));
+        metadataLocation -> TableMetadata.buildFromLocation(io(), metadataLocation).build());

Review Comment:
   Don't know about this, here we parse the metadata and then again copy over the structures in the builder. But ultimately when we read, we need to go through a single point in the builder for setting main if it doesn't exist and there is a current snapshot. I don't think just setting the ref when parsing would work because ultimately it needs to be encapsulated in an update even for operations which don't produce snapshots. @rdblue thoughts?



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887275295


##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -174,7 +174,7 @@ protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
   protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
                                              int numRetries) {
     refreshFromMetadataLocation(newLocation, shouldRetry, numRetries,
-        metadataLocation -> TableMetadataParser.read(io(), metadataLocation));
+        metadataLocation -> TableMetadata.buildFromLocation(io(), metadataLocation).build());

Review Comment:
   Don't know about this, here we parse the metadata and then again copy over the structures in the builder. But ultimately when we read, we need to go through a single point in the builder for setting main if it doesn't exist and there is a current snapshot. I don't think just setting the ref when parsing will work because then in the builder buildFrom(base) we won't be able to distinguish if main already existed in the metadata or if it was set when parsing; this distinction is needed to determine if we need to do the builder.setRef. @rdblue thoughts?



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887275892


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -752,7 +753,19 @@ private static Map<String, SnapshotRef> validateRefs(Long currentSnapshotId,
   }
 
   public static Builder buildFrom(TableMetadata base) {

Review Comment:
   We do it here because any operation interacting with the builder would use buildFrom and then the main ref can be set if it doesn't exist in base.



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887153265


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -529,6 +531,7 @@ public TableMetadata replaceProperties(Map<String, String> rawProperties) {
 
     return new Builder(this)
         .setProperties(updated)
+        .setRef(SnapshotRef.MAIN_BRANCH, SnapshotRef.branchBuilder(currentSnapshotId).build())

Review Comment:
   Good point, I should be setting this in the builder as that's the whole point of it. I'll update this



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887279993


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -752,7 +753,19 @@ private static Map<String, SnapshotRef> validateRefs(Long currentSnapshotId,
   }
 
   public static Builder buildFrom(TableMetadata base) {

Review Comment:
   Actually, I think I can just do this in build()



-- 
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


[GitHub] [iceberg] hililiwei commented on pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
hililiwei commented on PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#issuecomment-1170700851

   Is there any update here?


-- 
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


[GitHub] [iceberg] amogh-jahagirdar closed pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar closed pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state
URL: https://github.com/apache/iceberg/pull/4922


-- 
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


[GitHub] [iceberg] rdblue commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if there is no main but there is a current table state

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r912532893


##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -174,7 +174,7 @@ protected void refreshFromMetadataLocation(String newLocation, int numRetries) {
   protected void refreshFromMetadataLocation(String newLocation, Predicate<Exception> shouldRetry,
                                              int numRetries) {
     refreshFromMetadataLocation(newLocation, shouldRetry, numRetries,
-        metadataLocation -> TableMetadataParser.read(io(), metadataLocation));
+        metadataLocation -> TableMetadata.buildFromLocation(io(), metadataLocation).build());

Review Comment:
   I don't think we should change the metadata after reading it, so it is probably best to fix this up in the `TableMetadata` constructor rather than in the builder, which is used to produce new `TableMetadata` objects.



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887275892


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -752,7 +753,19 @@ private static Map<String, SnapshotRef> validateRefs(Long currentSnapshotId,
   }
 
   public static Builder buildFrom(TableMetadata base) {

Review Comment:
   We do it here because any operation interacting with the builder would use buildFrom and then the main ref can be set if it doesn't exist in base. I'll add a test if this is the direction we want to go



-- 
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


[GitHub] [iceberg] amogh-jahagirdar commented on a diff in pull request #4922: Core: Assign main branch to table's current snapshot if main could not be found

Posted by GitBox <gi...@apache.org>.
amogh-jahagirdar commented on code in PR #4922:
URL: https://github.com/apache/iceberg/pull/4922#discussion_r887275892


##########
core/src/main/java/org/apache/iceberg/TableMetadata.java:
##########
@@ -752,7 +753,19 @@ private static Map<String, SnapshotRef> validateRefs(Long currentSnapshotId,
   }
 
   public static Builder buildFrom(TableMetadata base) {

Review Comment:
   We do it here because any operation interacting with the builder would use buildFrom and then the main ref can be set if needed



-- 
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