[GitHub] [iceberg] jackye1995 commented on pull request #2957: Core: add TableMetadata update builder

[GitBox <gi...@apache.org>]

jackye1995 commented on pull request #2957:
URL: https://github.com/apache/iceberg/pull/2957#issuecomment-917857005


   @rdblue thanks for the review! Based on the conversation, I try to first introduce the builder for update only. To indicate the builder is for update only, I used name `TableMetadataUpdateBuilder` instead of `TableMetadataBuilder`.
   
   I was thinking about using the same builder for both update and new metadata, but there might be some unnecessary complications to mix those 2 together, because we need to set defaults for any null values when building new metadata, whereas we want to inherit whatever was in the base metadata without any defaults in an update. So my current stance is that we can introduce another builder later for new table metadata (when that is necessary), and evaluate if it is worth letting them sharing some common class or not.
   
   For the feature of accumulating states, I am leaving it out of this PR for now. I tried to implement it, but in update builder we need to recreate a list anyway for whatever we need to overwrite from the old metadata, so the benefit for building the list inside of builder is not obvious compared to building it outside and then overwrite the whole list. 


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