[
https://issues.apache.org/jira/browse/GOBBLIN-2157?focusedWorklogId=934986&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-934986
]
ASF GitHub Bot logged work on GOBBLIN-2157:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 16/Sep/24 23:44
Start Date: 16/Sep/24 23:44
Worklog Time Spent: 10m
Work Description: 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
Issue Time Tracking
-------------------
Worklog Id: (was: 934986)
Time Spent: 50m (was: 40m)
> Copy Iceberg table properties during iceberg table replication
> --------------------------------------------------------------
>
> Key: GOBBLIN-2157
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2157
> Project: Apache Gobblin
> Issue Type: Improvement
> Components: gobblin-core
> Reporter: William Lo
> Assignee: Abhishek Tiwari
> Priority: Major
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Currently, Iceberg table copies do not replicate any properties from the
> source table. This can be unintuitive as the expectation for most pipelines
> is that the full table data and properties are replicated on both the source
> and destination.
> We want to copy all the properties from the source table while maintaining
> any pre-existing properties that were created from the destination table.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
