phet commented on code in PR #4056:
URL: https://github.com/apache/gobblin/pull/4056#discussion_r1762088183


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergTable.java:
##########
@@ -212,8 +212,9 @@ public DatasetDescriptor getDatasetDescriptor(FileSystem 
fs) {
    * @param dstMetadata is null if destination {@link IcebergTable} is absent, 
in which case registration is skipped */
   protected void registerIcebergTable(TableMetadata srcMetadata, TableMetadata 
dstMetadata) {
     if (dstMetadata != null) {
-      // use current destination metadata as 'base metadata' and source as 
'updated metadata' while committing
-      this.tableOps.commit(dstMetadata, 
srcMetadata.replaceProperties(dstMetadata.properties()));
+      // Use current destination metadata as 'base metadata' and but update to 
source metadata when committing
+      // If any of the source table properties are deleted, they will be 
reflected in the destination table

Review Comment:
   "and but"
   
   how about - 
   > ...as 'base metadata', but commit the source-side metadata, to synchronize 
source-side property deletion over to the destination
   
   ?



##########
gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/iceberg/IcebergTableTest.java:
##########
@@ -181,6 +184,33 @@ public void 
testGetIncrementalSnapshotInfosIteratorRepeatedFiles() throws IOExce
     }
   }
 
+  /** Verify that registerIcebergTable will update existing table properties */
+  @Test
+  public void testNewTablePropertiesAreRegistered() throws Exception {
+    Map<String, String> srcTableProperties = Maps.newHashMap();
+    Map<String, String> destTableProperties = Maps.newHashMap();
+
+    srcTableProperties.put("newKey", "newValue");
+    // Expect the old value to be overwritten by the new value
+    srcTableProperties.put("testKey", "testValueNew");
+    destTableProperties.put("testKey", "testValueOld");
+    // Expect existing property values to be maintained if it does not exist 
on the source

Review Comment:
   comment looks out-of-date



-- 
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-unsubscr...@gobblin.apache.org

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

Reply via email to