RussellSpitzer commented on a change in pull request #1409:
URL: https://github.com/apache/iceberg/pull/1409#discussion_r483050254



##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
##########
@@ -218,6 +189,114 @@ public String toString() {
 
   protected abstract String defaultWarehouseLocation(TableIdentifier 
tableIdentifier);
 
+  protected class BaseMetastoreCatalogTableBuilder implements TableBuilder {
+    private final TableIdentifier identifier;
+    private final Schema schema;
+    private final ImmutableMap.Builder<String, String> propertiesBuilder = 
ImmutableMap.builder();
+    private PartitionSpec spec = PartitionSpec.unpartitioned();
+    private String location = null;
+
+    public BaseMetastoreCatalogTableBuilder(TableIdentifier identifier, Schema 
schema) {
+      Preconditions.checkArgument(isValidIdentifier(identifier), "Invalid 
table identifier: %s", identifier);
+
+      this.identifier = identifier;
+      this.schema = schema;
+    }
+
+    @Override
+    public TableBuilder withPartitionSpec(PartitionSpec newSpec) {
+      if (spec != null) {

Review comment:
       Why allow null here? We used to allow null for the parameter because you 
had to pass it, but now you don't have to pass it so maybe we should reject 
null?

##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
##########
@@ -218,6 +189,114 @@ public String toString() {
 
   protected abstract String defaultWarehouseLocation(TableIdentifier 
tableIdentifier);
 
+  protected class BaseMetastoreCatalogTableBuilder implements TableBuilder {
+    private final TableIdentifier identifier;
+    private final Schema schema;
+    private final ImmutableMap.Builder<String, String> propertiesBuilder = 
ImmutableMap.builder();
+    private PartitionSpec spec = PartitionSpec.unpartitioned();
+    private String location = null;
+
+    public BaseMetastoreCatalogTableBuilder(TableIdentifier identifier, Schema 
schema) {
+      Preconditions.checkArgument(isValidIdentifier(identifier), "Invalid 
table identifier: %s", identifier);
+
+      this.identifier = identifier;
+      this.schema = schema;
+    }
+
+    @Override
+    public TableBuilder withPartitionSpec(PartitionSpec newSpec) {
+      if (spec != null) {
+        this.spec = newSpec;
+      }
+      return this;
+    }
+
+    @Override
+    public TableBuilder withLocation(String newLocation) {
+      this.location = newLocation;
+      return this;
+    }
+
+    @Override
+    public TableBuilder withProperties(Map<String, String> properties) {
+      if (properties != null) {

Review comment:
       Same null question 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.

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