nsivabalan commented on code in PR #13616:
URL: https://github.com/apache/hudi/pull/13616#discussion_r2229827053


##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -321,14 +321,13 @@ public class HoodieTableConfig extends HoodieConfig {
       .withDocumentation("This property when set, will define how two versions 
of the record will be "
           + "merged together where the later contains only partial set of 
values and not entire record.");
 
-  public static final ConfigProperty<String> MERGE_PROPERTIES = ConfigProperty
-      .key("hoodie.table.merge.properties")
+  public static final ConfigProperty<String> MERGE_CUSTOM_PROPERTY_PREFIX = 
ConfigProperty
+      .key("hoodie.merge.custom.property.prefix")
       .noDefaultValue()
       .sinceVersion("1.1.0")
-      .withDocumentation("The value of this property is in the format of 
'K1=V1,K2=V2,...,Ki=Vi,...'. "
-          + "Each (Ki, Vi) pair represents a property used during merge 
scenarios. "
-          + "Some merge mode might need some writer properties that are 
required for readers to function as expected. "
-          + "Hudi stores those properties here so readers can set them while 
reading.");
+      .withDocumentation("Some merge mode might need writer properties that 
are required for readers "

Review Comment:
   we need to fix the docs



##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -321,14 +321,13 @@ public class HoodieTableConfig extends HoodieConfig {
       .withDocumentation("This property when set, will define how two versions 
of the record will be "
           + "merged together where the later contains only partial set of 
values and not entire record.");
 
-  public static final ConfigProperty<String> MERGE_PROPERTIES = ConfigProperty
-      .key("hoodie.table.merge.properties")
+  public static final ConfigProperty<String> MERGE_CUSTOM_PROPERTY_PREFIX = 
ConfigProperty
+      .key("hoodie.merge.custom.property.prefix")

Review Comment:
   `hoodie.table.merge.properties.`
   and why do we need this to be a Config Property in itself. 
   we just need to declare this as constant variable here. 
   
   Lets see how this pan out:
   
   In HoodieTableMetaClient.TableBuilder, lets add a new method called
   `addCustomMergeProperties(Map<String, String> customProperties)` 
   
   in the impl, we process these properties and write them out as N diff 
properties w/ the prefix (constant) string declared 
("hoodie.table.merge.properties."). 
   Something like 
   ```
   hoodie.table.merge.properties.k1 = v1
   hoodie.table.merge.properties.k2 = v2
   ```
   And this will get serialized to hoodie.properties. 
   
   We should have a getter method in 
HoodieTableConfig.getCustomMergeProperties(), 
   we should process all properties w/ above prefix and return a Map<String, 
String> 
   
   
   
   
   



##########
hudi-common/src/main/java/org/apache/hudi/common/util/ConfigUtils.java:
##########
@@ -642,4 +642,42 @@ public static HoodieConfig 
getReaderConfigs(StorageConfiguration<?> storageConf)
   public static TypedProperties loadGlobalProperties() {
     return ((PropertiesConfig) 
ReflectionUtils.loadClass("org.apache.hudi.common.config.DFSPropertiesConfiguration")).getGlobalProperties();
   }
+
+  /**
+   * Extract all properties whose keys start with a given prefix.
+   * E.g., if the prefix is "a.b.c", and the props contain:
+   * "a.b.c.K1=V1", "a.b.c.K2=V2", "a.b.c.K3=V3".
+   * Then the output is:
+   * Map(K1->V1, K2->V2, K3->V3).
+   */
+  public static Map<String, String> extractWithPrefix(TypedProperties props, 
String prefix) {
+    if (props == null || props.isEmpty()) {
+      return new HashMap<>();
+    }
+    int prefixLength = prefix.length();
+    int dotOffset = prefixLength + 1; // +1 for the dot after prefix
+
+    Map<String, String> mergeProperties = new HashMap<>();
+    for (Map.Entry<Object, Object> entry : props.entrySet()) {
+      String key = entry.getKey().toString();
+      // Early exit if key is shorter than prefix or doesn't start with prefix
+      if (key.length() <= prefixLength || !key.startsWith(prefix)) {
+        continue;
+      }
+      // Check if the character after prefix is a dot
+      if (key.charAt(prefixLength) != '.') {

Review Comment:
   we should include "." in the prefix only and simplify this



##########
hudi-common/src/main/java/org/apache/hudi/common/util/ConfigUtils.java:
##########
@@ -642,4 +642,42 @@ public static HoodieConfig 
getReaderConfigs(StorageConfiguration<?> storageConf)
   public static TypedProperties loadGlobalProperties() {
     return ((PropertiesConfig) 
ReflectionUtils.loadClass("org.apache.hudi.common.config.DFSPropertiesConfiguration")).getGlobalProperties();
   }
+
+  /**
+   * Extract all properties whose keys start with a given prefix.
+   * E.g., if the prefix is "a.b.c", and the props contain:
+   * "a.b.c.K1=V1", "a.b.c.K2=V2", "a.b.c.K3=V3".
+   * Then the output is:
+   * Map(K1->V1, K2->V2, K3->V3).
+   */
+  public static Map<String, String> extractWithPrefix(TypedProperties props, 
String prefix) {

Review Comment:
   The getter looks good



-- 
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]

Reply via email to