[GitHub] [gobblin] vikrambohra commented on a change in pull request #3426: [GOBBLIN-1574] Added whitelist for iceberg tables to add new partitio…

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

vikrambohra commented on a change in pull request #3426:
URL: https://github.com/apache/gobblin/pull/3426#discussion_r751882526



##########
File path: gobblin-iceberg/src/main/java/org/apache/gobblin/iceberg/writer/IcebergMetadataWriter.java
##########
@@ -284,9 +287,8 @@ private Long getAndPersistCurrentWatermark(TableIdentifier tid, String topicPart
    * information increases the memory footprints, therefore we would like to flush them eagerly).
    */
   public void write(GobblinMetadataChangeEvent gmce, Map<String, Collection<HiveSpec>> newSpecsMap,
-      Map<String, Collection<HiveSpec>> oldSpecsMap, HiveSpec tableSpec) throws IOException {
-    TableIdentifier tid = TableIdentifier.of(tableSpec.getTable().getDbName(), tableSpec.getTable().getTableName());
-    TableMetadata tableMetadata = tableMetadataMap.computeIfAbsent(tid, t -> new TableMetadata());
+      Map<String, Collection<HiveSpec>> oldSpecsMap, HiveSpec tableSpec, TableMetadata tableMetadata) throws IOException {

Review comment:
       tid is an iceberg concept with namespace and name rather than db and table. Namespace can be dbName or a multilevel name like hivedb.dbname which could lead to errors. Other option is to pass tableSpec (which has db and table name) to addFiles method. Parsing db and table name everytime we need is not ideal.




-- 
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: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org