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