Bug fix: Complex type transformer should not be created for empty con…

…fig (apache#8600) * Bug fix: Complex type transformer should not be created for empty configs * Add unit tests
sanjayprak · Apr 27, 2022 · 6484742 · 6484742
1 parent 89836a5
commit 6484742
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 15 deletions.
diff --git a/...rc/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java b/...rc/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
@@ -136,7 +136,8 @@ private static String parseDelimiter(TableConfig tableConfig) {
    */
   @Nullable
   public static ComplexTypeTransformer getComplexTypeTransformer(TableConfig tableConfig) {
-    if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) {
+    if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null
+    && tableConfig.getIngestionConfig().getComplexTypeConfig().getFieldsToUnnest() != null) {
       return new ComplexTypeTransformer(tableConfig);
     }
     return null;

diff --git a/...est/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformerTest.java b/...est/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformerTest.java
@@ -22,12 +22,17 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
 import org.apache.pinot.spi.config.table.ingestion.ComplexTypeConfig;
+import org.apache.pinot.spi.config.table.ingestion.IngestionConfig;
 import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
@@ -298,8 +303,8 @@ public void testConvertCollectionToString() {
     // {
     //   "array":"[1,2]"
     // }
-    transformer = new ComplexTypeTransformer(Arrays.asList(), ".",
-            ComplexTypeConfig.CollectionNotUnnestedToJson.ALL, new HashMap<>());
+    transformer = new ComplexTypeTransformer(Arrays.asList(), ".", ComplexTypeConfig.CollectionNotUnnestedToJson.ALL,
+        new HashMap<>());
     genericRow = new GenericRow();
     array = new Object[]{1, 2};
     genericRow.putValue("array", array);
@@ -344,8 +349,8 @@ public void testConvertCollectionToString() {
     array1[0] = ImmutableMap.of("b", "v1");
     map.put("array1", array1);
     genericRow.putValue("t", map);
-    transformer = new ComplexTypeTransformer(Arrays.asList(), ".",
-            ComplexTypeConfig.CollectionNotUnnestedToJson.NONE, new HashMap<>());
+    transformer = new ComplexTypeTransformer(Arrays.asList(), ".", ComplexTypeConfig.CollectionNotUnnestedToJson.NONE,
+        new HashMap<>());
     transformer.transform(genericRow);
     Assert.assertTrue(ComplexTypeTransformer.isArray(genericRow.getValue("t.array1")));
   }
@@ -355,8 +360,8 @@ public void testRenamePrefixes() {
     HashMap<String, String> prefixesToRename = new HashMap<>();
     prefixesToRename.put("map1.", "");
     prefixesToRename.put("map2", "test");
-    ComplexTypeTransformer transformer = new ComplexTypeTransformer(new ArrayList<>(), ".",
-            DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
+    ComplexTypeTransformer transformer =
+        new ComplexTypeTransformer(new ArrayList<>(), ".", DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
 
     GenericRow genericRow = new GenericRow();
     genericRow.putValue("a", 1L);
@@ -370,8 +375,7 @@ public void testRenamePrefixes() {
     // name conflict where there becomes duplicate field names after renaming
     prefixesToRename = new HashMap<>();
     prefixesToRename.put("test.", "");
-    transformer = new ComplexTypeTransformer(new ArrayList<>(), ".",
-            DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
+    transformer = new ComplexTypeTransformer(new ArrayList<>(), ".", DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
     genericRow = new GenericRow();
     genericRow.putValue("a", 1L);
     genericRow.putValue("test.a", 2L);
@@ -385,8 +389,7 @@ public void testRenamePrefixes() {
     // name conflict where there becomes an empty field name after renaming
     prefixesToRename = new HashMap<>();
     prefixesToRename.put("test", "");
-    transformer = new ComplexTypeTransformer(new ArrayList<>(), ".",
-            DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
+    transformer = new ComplexTypeTransformer(new ArrayList<>(), ".", DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
     genericRow = new GenericRow();
     genericRow.putValue("a", 1L);
     genericRow.putValue("test", 2L);
@@ -399,8 +402,7 @@ public void testRenamePrefixes() {
 
     // case where nothing gets renamed
     prefixesToRename = new HashMap<>();
-    transformer = new ComplexTypeTransformer(new ArrayList<>(), ".",
-            DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
+    transformer = new ComplexTypeTransformer(new ArrayList<>(), ".", DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
     genericRow = new GenericRow();
     genericRow.putValue("a", 1L);
     genericRow.putValue("test", 2L);
@@ -414,8 +416,8 @@ public void testPrefixesToRename() {
     HashMap<String, String> prefixesToRename = new HashMap<>();
     prefixesToRename.put("map1.", "");
     prefixesToRename.put("map2", "test");
-    ComplexTypeTransformer transformer = new ComplexTypeTransformer(new ArrayList<>(), ".",
-            DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
+    ComplexTypeTransformer transformer =
+        new ComplexTypeTransformer(new ArrayList<>(), ".", DEFAULT_COLLECTION_TO_JSON_MODE, prefixesToRename);
 
     // test flatten root-level tuples
     GenericRow genericRow = new GenericRow();
@@ -438,4 +440,26 @@ public void testPrefixesToRename() {
     Assert.assertEquals(genericRow.getValue("im1.bb"), "u");
     Assert.assertEquals(genericRow.getValue("test.c"), 3);
   }
+
+  @Test
+  public void getComplexTypeTransformerTest() {
+    ComplexTypeConfig complexTypeConfigWithNullFields = new ComplexTypeConfig(null, null, null, null);
+
+    IngestionConfig ingestionConfig = new IngestionConfig(null, null, null, null, complexTypeConfigWithNullFields);
+    TableConfig tableConfig =
+        new TableConfigBuilder(TableType.OFFLINE).setTableName("test_null").setIngestionConfig(ingestionConfig).build();
+
+    ComplexTypeTransformer complexTypeTransformer = ComplexTypeTransformer.getComplexTypeTransformer(tableConfig);
+    Assert.assertNull(complexTypeTransformer);
+
+    List<String> fieldToUnnest = Collections.singletonList("foo_bar");
+    ComplexTypeConfig complexTypeConfigWithNonNullField =
+        new ComplexTypeConfig(fieldToUnnest, null, null, null);
+    ingestionConfig = new IngestionConfig(null, null, null, null, complexTypeConfigWithNonNullField);
+    tableConfig =
+        new TableConfigBuilder(TableType.OFFLINE).setTableName("test_non_null").setIngestionConfig(ingestionConfig)
+            .build();
+    complexTypeTransformer = ComplexTypeTransformer.getComplexTypeTransformer(tableConfig);
+    Assert.assertNotNull(complexTypeTransformer);
+  }
 }