Fixed two bugs in FastDeserializerGenerator (#46)

1. fixed null exception when updating actual exceptions for already generated record method; 2. corrected value action symbols iteration order for nested Map; Co-authored-by: Bingfeng Xia <[email protected]>
linkedin · May 6, 2020 · 84432ee · 84432ee
1 parent e49d36e
commit 84432ee
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 4 deletions.
diff --git a/avro-fastserde/src/main/java/com/linkedin/avro/fastserde/FastDeserializerGenerator.java b/avro-fastserde/src/main/java/com/linkedin/avro/fastserde/FastDeserializerGenerator.java
@@ -363,9 +363,11 @@ private void processRecord(JVar recordSchemaVar, String recordName, final Schema
 
   private void updateActualExceptions(JMethod method) {
     Set<Class<? extends Exception>> exceptionFromMethod = exceptionFromMethodMap.get(method);
-    for (Class<? extends Exception> exceptionClass : exceptionFromMethod) {
-      method._throws(exceptionClass);
-      schemaAssistant.getExceptionsFromStringable().add(exceptionClass);
+    if (exceptionFromMethod != null) {
+      for (Class<? extends Exception> exceptionClass : exceptionFromMethod) {
+        method._throws(exceptionClass);
+        schemaAssistant.getExceptionsFromStringable().add(exceptionClass);
+      }
     }
   }
 
@@ -696,9 +698,20 @@ private void processMap(JVar mapSchemaVar, final String name, final Schema mapSc
       JBlock parentBody, FieldAction action, BiConsumer<JBlock, JExpression> putMapIntoParent,
       Supplier<JExpression> reuseSupplier) {
 
+    /**
+     * Determine the action symbol for Map value. {@link ResolvingGrammarGenerator} generates
+     * resolving grammar symbols with reversed order of production sequence. If this symbol is
+     * a terminal, its production list will be <tt>null</tt>. Otherwise the production list
+     * holds the the sequence of the symbols that forms this symbol.
+     *
+     * The {@link FastDeserializerGenerator.generateDeserializer} tries to proceed as a depth-first,
+     * left-to-right traversal of the schema. So for a nested Map, we need to iterate production list
+     * in reverse order to get the correct "map-end" symbol of internal Maps.
+     */
     if (action.getShouldRead()) {
       Symbol valuesActionSymbol = null;
-      for (Symbol symbol : action.getSymbol().production) {
+      for (int i = action.getSymbol().production.length - 1; i >= 0; --i) {
+        Symbol symbol = action.getSymbol().production[i];
         if (Symbol.Kind.REPEATER.equals(symbol.kind) && "map-end".equals(
             getSymbolPrintName(((Symbol.Repeater) symbol).end))) {
           valuesActionSymbol = symbol;

diff --git a/...serde/src/test/java/com/linkedin/avro/fastserde/FastGenericDeserializerGeneratorTest.java b/...serde/src/test/java/com/linkedin/avro/fastserde/FastGenericDeserializerGeneratorTest.java
@@ -677,6 +677,70 @@ public void shouldReadMapOfRecords(Boolean whetherUseFastDeserializer) {
     Assert.assertEquals(new Utf8("abc"), map.get(new Utf8("2")).get("field"));
   }
 
+  @Test(groups = {"deserializationTest"}, dataProvider = "SlowFastDeserializer")
+  public void shouldReadNestedMap(Boolean whetherUseFastDeserializer) {
+    // given
+    Schema nestedMapSchema = createRecord("record", createMapFieldSchema(
+        "mapField", Schema.createMap(Schema.createArray(Schema.create(Schema.Type.INT)))));
+
+    Map<String, List> value = new HashMap<>();
+    value.put("subKey1", Arrays.asList(1));
+    value.put("subKey2", Arrays.asList(2));
+    Map<String, Map<String, List>> mapField = new HashMap<>();
+    mapField.put("key1", value);
+
+    GenericData.Record recordData = new GenericData.Record(nestedMapSchema);
+    recordData.put("mapField", mapField);
+
+    // when
+    GenericData.Record decodedRecord = null;
+    if (whetherUseFastDeserializer) {
+      decodedRecord = decodeRecordFast(nestedMapSchema, nestedMapSchema,
+          FastSerdeTestsSupport.genericDataAsDecoder(recordData, nestedMapSchema));
+    } else {
+      decodedRecord = decodeRecordSlow(nestedMapSchema, nestedMapSchema,
+          FastSerdeTestsSupport.genericDataAsDecoder(recordData, nestedMapSchema));
+    }
+
+    // then
+    Object decodedMapField = decodedRecord.get("mapField");
+    Assert.assertEquals("{key1={subKey1=[1], subKey2=[2]}}", decodedMapField.toString());
+    Assert.assertTrue(decodedMapField instanceof Map);
+    Object subMap = ((Map) decodedMapField).get(new Utf8("key1"));
+    Assert.assertTrue(subMap instanceof Map);
+    Assert.assertEquals(Arrays.asList(1), ((List) ((Map) subMap).get(new Utf8("subKey1"))));
+    Assert.assertEquals(Arrays.asList(2), ((List) ((Map) subMap).get(new Utf8("subKey2"))));
+  }
+
+  @Test(groups = {"deserializationTest"}, dataProvider = "SlowFastDeserializer")
+  public void shouldReadRecursiveUnionRecord(Boolean whetherUseFastDeserializer) {
+    // given
+    Schema unionRecordSchema = Schema.parse("{\"type\":\"record\",\"name\":\"recordName\",\"namespace\":\"com.linkedin.avro.fastserde.generated.avro\",\"fields\":[{\"name\":\"strField\",\"type\":\"string\"},{\"name\":\"unionField\",\"type\":[\"null\",\"recordName\"]}]}");
+
+    GenericData.Record recordData = new GenericData.Record(unionRecordSchema);
+    recordData.put("strField", "foo");
+
+    GenericData.Record unionField = new GenericData.Record(unionRecordSchema);
+    unionField.put("strField", "bar");
+    recordData.put("unionField", unionField);
+
+    // when
+    GenericData.Record decodedRecord = null;
+    if (whetherUseFastDeserializer) {
+      decodedRecord = decodeRecordFast(unionRecordSchema, unionRecordSchema,
+          FastSerdeTestsSupport.genericDataAsDecoder(recordData, unionRecordSchema));
+    } else {
+      decodedRecord = decodeRecordSlow(unionRecordSchema, unionRecordSchema,
+          FastSerdeTestsSupport.genericDataAsDecoder(recordData, unionRecordSchema));
+    }
+
+    // then
+    Assert.assertEquals(new Utf8("foo"), decodedRecord.get("strField"));
+    Object decodedUnionField = decodedRecord.get("unionField");
+    Assert.assertTrue(decodedUnionField instanceof GenericData.Record);
+    Assert.assertEquals(new Utf8("bar"), ((GenericData.Record) decodedUnionField).get("strField"));
+  }
+
   public <T> T decodeRecordFast(Schema writerSchema, Schema readerSchema, Decoder decoder) {
     FastDeserializer<T> deserializer =
         new FastGenericDeserializerGenerator<T>(writerSchema, readerSchema, tempDir, classLoader,