rajarshisarkar commented on a change in pull request #4011:
URL: https://github.com/apache/iceberg/pull/4011#discussion_r807720897



##########
File path: core/src/main/java/org/apache/iceberg/util/PropertyUtil.java
##########
@@ -70,4 +72,16 @@ public static String propertyAsString(Map<String, String> 
properties,
     }
     return defaultValue;
   }
+
+  public static Map<String, String> propertiesWithPrefix(Map<String, String> 
properties,

Review comment:
       The method name doesn't suggest that we replace the keys which starts 
with the prefix.

##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
##########
@@ -106,7 +114,7 @@ public String toString() {
   protected class BaseMetastoreCatalogTableBuilder implements TableBuilder {
     private final TableIdentifier identifier;
     private final Schema schema;
-    private final ImmutableMap.Builder<String, String> propertiesBuilder = 
ImmutableMap.builder();
+    private final Map<String, String> propertiesBuilder = Maps.newHashMap();

Review comment:
       How about `properties`?

##########
File path: core/src/main/java/org/apache/iceberg/util/PropertyUtil.java
##########
@@ -70,4 +72,16 @@ public static String propertyAsString(Map<String, String> 
properties,
     }
     return defaultValue;
   }
+
+  public static Map<String, String> propertiesWithPrefix(Map<String, String> 
properties,
+                                                         String prefix) {
+    if (properties == null || properties.isEmpty()) {
+      return Collections.emptyMap();
+    }
+
+    return properties.entrySet().stream()
+        .filter(e -> e.getKey().startsWith(prefix))
+        .collect(Collectors.toMap(
+            e -> e.getKey().replace(prefix, ""), Map.Entry::getValue));

Review comment:
       We can use `replaceFirst` if we are interested only in replacing the 
prefix.

##########
File path: core/src/main/java/org/apache/iceberg/BaseMetastoreCatalog.java
##########
@@ -178,8 +187,8 @@ public Transaction createTransaction() {
       }
 
       String baseLocation = location != null ? location : 
defaultWarehouseLocation(identifier);
-      Map<String, String> properties = propertiesBuilder.build();
-      TableMetadata metadata = TableMetadata.newTableMetadata(schema, spec, 
sortOrder, baseLocation, properties);
+      propertiesBuilder.putAll(tableOverrideProperties());
+      TableMetadata metadata = TableMetadata.newTableMetadata(schema, spec, 
sortOrder, baseLocation, propertiesBuilder);

Review comment:
       Is it now okay to pass `Map` instead of `ImmutableMap` while creating  
`TableMetadata` object?




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