yihua commented on code in PR #13519:
URL: https://github.com/apache/hudi/pull/13519#discussion_r2263509773
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -527,7 +534,8 @@ public static void recover(HoodieStorage storage,
StoragePath metadataFolder) th
recoverIfNeeded(storage, cfgPath, backupCfgPath);
}
- private static void modify(HoodieStorage storage, StoragePath
metadataFolder, Properties modifyProps, BiConsumer<Properties, Properties>
modifyFn) {
+ private static void modify(HoodieStorage storage, StoragePath
metadataFolder, Properties modifyProps, BiConsumer<Properties, Properties>
modifyFn,
+ Set<String> propsToDelete) {
Review Comment:
```suggestion
private static void modify(HoodieStorage storage, StoragePath
metadataFolder, Properties propsToUpdate, Set<String> propsToDelete) {
```
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -230,6 +231,12 @@ public class HoodieTableConfig extends HoodieConfig {
.withDocumentation("Payload class to use for performing merges,
compactions, i.e merge delta logs with current base file and then "
+ " produce a new base file.");
+ public static final ConfigProperty<String> LEGACY_PAYLOAD_CLASS_NAME =
ConfigProperty
+ .key("hoodie.legacy.payload.class")
Review Comment:
```suggestion
.key("hoodie.table.legacy.payload.class")
```
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -582,13 +591,18 @@ private static void deleteFile(HoodieStorage storage,
StoragePath cfgPath) throw
*/
public static void update(HoodieStorage storage, StoragePath metadataFolder,
Properties updatedProps) {
- modify(storage, metadataFolder, updatedProps,
ConfigUtils::upsertProperties);
+ modify(storage, metadataFolder, updatedProps,
ConfigUtils::upsertProperties, Collections.EMPTY_SET);
+ }
+
+ public static void updateDeleteProps(HoodieStorage storage, StoragePath
metadataFolder,
+ Properties updatedProps, Set<String>
propstoDelete) {
+ modify(storage, metadataFolder, updatedProps,
ConfigUtils::upsertProperties, propstoDelete);
}
public static void delete(HoodieStorage storage, StoragePath metadataFolder,
Set<String> deletedProps) {
Properties props = new Properties();
deletedProps.forEach(p -> props.setProperty(p, ""));
- modify(storage, metadataFolder, props, ConfigUtils::deleteProperties);
+ modify(storage, metadataFolder, props, ConfigUtils::deleteProperties,
Collections.EMPTY_SET);
Review Comment:
nit: could these methods be refactored so that properties to update and
deleted are separated? Currently deletes can either go through `modifyProps`
and `propsToDelete` which are confusing.
##########
hudi-common/src/main/java/org/apache/hudi/common/model/AWSDmsAvroPayload.java:
##########
@@ -45,6 +45,7 @@
public class AWSDmsAvroPayload extends OverwriteWithLatestAvroPayload {
public static final String OP_FIELD = "Op";
+ public static final String D_VALUE = "D";
Review Comment:
Use `OP_DELETE` in `isDMSDeleteRecord`?
```suggestion
public static final String OP_DELETE = "D";
```
--
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]
