This is an automated email from the ASF dual-hosted git repository.

codope pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git


The following commit(s) were added to refs/heads/master by this push:
     new 6447cf2d38e [HUDI-6400] Fail when merger class not found (#8998)
6447cf2d38e is described below

commit 6447cf2d38e1e6f45eb69b275467c2a1174b799e
Author: Nicolas Paris <[email protected]>
AuthorDate: Mon Aug 7 13:11:11 2023 +0200

    [HUDI-6400] Fail when merger class not found (#8998)
    
    Currently when the user's specified class does not exists, then this
    silently fall back to the default merger. It can silently corrupt the data
    by applying wrong logic. This commit fixes by failing when merge class
    not found, instead of using default merger that might corrupt the data.
---
 .../org/apache/hudi/common/util/HoodieRecordUtils.java | 18 ++++++------------
 .../apache/hudi/common/util/TestHoodieRecordUtils.java | 12 +++++++++++-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git 
a/hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java 
b/hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java
index 4bd7ffccbab..7eea31bea76 100644
--- 
a/hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java
+++ 
b/hudi-common/src/main/java/org/apache/hudi/common/util/HoodieRecordUtils.java
@@ -43,6 +43,7 @@ public class HoodieRecordUtils {
   private static final Logger LOG = 
LoggerFactory.getLogger(HoodieRecordUtils.class);
 
   private static final Map<String, Object> INSTANCE_CACHE = new HashMap<>();
+
   static {
     INSTANCE_CACHE.put(HoodieAvroRecordMerger.class.getName(), 
HoodieAvroRecordMerger.INSTANCE);
   }
@@ -58,7 +59,7 @@ public class HoodieRecordUtils {
           recordMerger = (HoodieRecordMerger) INSTANCE_CACHE.get(mergerClass);
           if (null == recordMerger) {
             recordMerger = (HoodieRecordMerger) 
ReflectionUtils.loadClass(mergerClass,
-                new Object[]{});
+                new Object[] {});
             INSTANCE_CACHE.put(mergerClass, recordMerger);
           }
         }
@@ -73,19 +74,12 @@ public class HoodieRecordUtils {
    * Instantiate a given class with a record merge.
    */
   public static HoodieRecordMerger createRecordMerger(String basePath, 
EngineType engineType,
-      List<String> mergerClassList, String recordMergerStrategy) {
+                                                      List<String> 
mergerClassList, String recordMergerStrategy) {
     if (mergerClassList.isEmpty() || 
HoodieTableMetadata.isMetadataTable(basePath)) {
       return HoodieAvroRecordMerger.INSTANCE;
     } else {
       return mergerClassList.stream()
-          .map(clazz -> {
-            try {
-              return loadRecordMerger(clazz);
-            } catch (HoodieException e) {
-              LOG.warn(String.format("Unable to init %s", clazz), e);
-              return null;
-            }
-          })
+          .map(clazz -> loadRecordMerger(clazz))
           .filter(Objects::nonNull)
           .filter(merger -> 
merger.getMergingStrategy().equals(recordMergerStrategy))
           .filter(merger -> recordTypeCompatibleEngine(merger.getRecordType(), 
engineType))
@@ -98,8 +92,8 @@ public class HoodieRecordUtils {
    * Instantiate a given class with an avro record payload.
    */
   public static <T extends HoodieRecordPayload> T loadPayload(String 
recordPayloadClass,
-      Object[] payloadArgs,
-      Class<?>... constructorArgTypes) {
+                                                              Object[] 
payloadArgs,
+                                                              Class<?>... 
constructorArgTypes) {
     try {
       return (T) 
ReflectionUtils.getClass(recordPayloadClass).getConstructor(constructorArgTypes)
           .newInstance(payloadArgs);
diff --git 
a/hudi-common/src/test/java/org/apache/hudi/common/util/TestHoodieRecordUtils.java
 
b/hudi-common/src/test/java/org/apache/hudi/common/util/TestHoodieRecordUtils.java
index 7cc1f2b0b8f..f06670cc76d 100644
--- 
a/hudi-common/src/test/java/org/apache/hudi/common/util/TestHoodieRecordUtils.java
+++ 
b/hudi-common/src/test/java/org/apache/hudi/common/util/TestHoodieRecordUtils.java
@@ -19,13 +19,17 @@
 package org.apache.hudi.common.util;
 
 import org.apache.avro.generic.GenericRecord;
+
 import org.apache.hudi.common.model.DefaultHoodieRecordPayload;
 import org.apache.hudi.common.model.HoodieAvroRecordMerger;
 import org.apache.hudi.common.model.HoodieRecordMerger;
 import org.apache.hudi.common.model.HoodieRecordPayload;
+import org.apache.hudi.exception.HoodieException;
+
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class TestHoodieRecordUtils {
 
@@ -38,10 +42,16 @@ class TestHoodieRecordUtils {
     assertEquals(recordMerger1, recordMerger2);
   }
 
+  @Test
+  void loadHoodieMergeWithWrongMerger() {
+    String mergeClassName = "wrong.package.MergerName";
+    assertThrows(HoodieException.class, () -> 
HoodieRecordUtils.loadRecordMerger(mergeClassName));
+  }
+
   @Test
   void loadPayload() {
     String payloadClassName = DefaultHoodieRecordPayload.class.getName();
-    HoodieRecordPayload payload = 
HoodieRecordUtils.loadPayload(payloadClassName, new Object[]{null, 0}, 
GenericRecord.class, Comparable.class);
+    HoodieRecordPayload payload = 
HoodieRecordUtils.loadPayload(payloadClassName, new Object[] {null, 0}, 
GenericRecord.class, Comparable.class);
     assertEquals(payload.getClass().getName(), payloadClassName);
   }
 }
\ No newline at end of file

Reply via email to