nastra commented on code in PR #8147:
URL: https://github.com/apache/iceberg/pull/8147#discussion_r1274919668


##########
core/src/main/java/org/apache/iceberg/MetadataUpdate.java:
##########
@@ -23,11 +23,16 @@
 import java.util.Map;
 import java.util.Set;
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.view.ImmutableViewMetadata;
+import org.apache.iceberg.view.ViewVersion;
 
-/** Represents a change to table metadata. */
+/** Represents a change to table/view metadata. */
 public interface MetadataUpdate extends Serializable {
   void applyTo(TableMetadata.Builder metadataBuilder);
 
+  default void applyTo(ImmutableViewMetadata.Builder viewMetadataBuilder) {}

Review Comment:
   > Is it possible to not attach this to an Immutable interface? I'd rather 
have an interface that we have more control over.
   
   That might be difficult without having to define a separate 
`ViewMetadataBuilder` API that would essentially have the same API as 
`ImmutableViewMetadata.Builder`. This wrapping builder could also take care of 
adding the metadata changes and could look like this:
   
   ```
   class Builder {
       private final ImmutableViewMetadata.Builder builder = 
ImmutableViewMetadata.builder();
   
       private Builder() {}
   
       public Builder formatVersion(int formatVersion) {
         builder.formatVersion(formatVersion);
         builder.addChanges(new 
MetadataUpdate.UpgradeFormatVersion(formatVersion));
         return this;
       }
   
       public Builder location(String location) {
         builder.location(location);
         builder.addChanges(new MetadataUpdate.SetLocation(location));
         return this;
       }
   
       public Builder currentVersionId(int currentVersionId) {
         builder.currentVersionId(currentVersionId);
         builder.addChanges(new 
MetadataUpdate.SetCurrentViewVersion(currentVersionId));
         return this;
       }
   
       public Builder currentSchemaId(Integer currentSchemaId) {
         builder.currentSchemaId(currentSchemaId);
         builder.addChanges(new 
MetadataUpdate.SetCurrentSchema(currentSchemaId));
         return this;
       }
   
       public Builder addSchemas(Schema schema) {
         builder.addSchemas(schema);
         builder.addChanges(new MetadataUpdate.AddSchema(schema, 
schema.highestFieldId()));
         return this;
       }
   
       public Builder addSchemas(Schema... schemas) {
         builder.addSchemas(schemas);
         for (Schema schema : schemas) {
           builder.addChanges(new MetadataUpdate.AddSchema(schema, 
schema.highestFieldId()));
         }
         return this;
       }
   
       public Builder schemas(Iterable<? extends Schema> schemas) {
         builder.schemas(schemas);
         for (Schema schema : schemas) {
           builder.addChanges(new MetadataUpdate.AddSchema(schema, 
schema.highestFieldId()));
         }
         return this;
       }
   
       public Builder addAllSchemas(Iterable<? extends Schema> schemas) {
         builder.addAllSchemas(schemas);
         for (Schema schema : schemas) {
           builder.addChanges(new MetadataUpdate.AddSchema(schema, 
schema.highestFieldId()));
         }
         return this;
       }
   
       public Builder addVersions(ViewVersion version) {
         builder.addVersions(version);
         builder.addChanges(new MetadataUpdate.AddViewVersion(version));
         return this;
       }
   
       public Builder addVersions(ViewVersion... versions) {
         builder.addVersions(versions);
         for (ViewVersion version : versions) {
           builder.addChanges(new MetadataUpdate.AddViewVersion(version));
         }
         return this;
       }
   
       public Builder versions(Iterable<? extends ViewVersion> versions) {
         builder.versions(versions);
         for (ViewVersion version : versions) {
           builder.addChanges(new MetadataUpdate.AddViewVersion(version));
         }
         return this;
       }
   
       public Builder addAllVersions(Iterable<? extends ViewVersion> versions) {
         builder.addAllVersions(versions);
         for (ViewVersion version : versions) {
           builder.addChanges(new MetadataUpdate.AddViewVersion(version));
         }
         return this;
       }
   
       public Builder addHistory(ViewHistoryEntry history) {
         builder.addHistory(history);
         return this;
       }
   
       public Builder addHistory(ViewHistoryEntry... history) {
         builder.addHistory(history);
         return this;
       }
   
       public Builder history(Iterable<? extends ViewHistoryEntry> history) {
         builder.history(history);
         return this;
       }
   
       public Builder addAllHistory(Iterable<? extends ViewHistoryEntry> 
history) {
         builder.addAllHistory(history);
         return this;
       }
   
       public Builder putProperties(String key, String value) {
         builder.putProperties(key, value);
         builder.addChanges(new 
MetadataUpdate.SetProperties(ImmutableMap.of(key, value)));
         return this;
       }
   
       public Builder putProperties(Map.Entry<String, String> entry) {
         builder.putProperties(entry);
         builder.addChanges(
             new MetadataUpdate.SetProperties(ImmutableMap.of(entry.getKey(), 
entry.getValue())));
         return this;
       }
   
       public Builder properties(Map<String, String> properties) {
         builder.properties(properties);
         builder.addChanges(new MetadataUpdate.SetProperties(properties));
         return this;
       }
   
       public Builder putAllProperties(Map<String, String> properties) {
         builder.putAllProperties(properties);
         builder.addChanges(new MetadataUpdate.SetProperties(properties));
         return this;
       }
   
       public Builder removeProperties(Set<String> propertiesToRemove) {
         Map<String, String> properties = 
Maps.newHashMap(builder.build().properties());
         propertiesToRemove.forEach(properties::remove);
         builder.properties(properties);
         builder.addChanges(new 
MetadataUpdate.RemoveProperties(propertiesToRemove));
         return this;
       }
   
       ViewMetadata build() {
         return builder.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: [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]

Reply via email to