[GitHub] [hudi] umehrot2 commented on a change in pull request #1953: [HUDI-1181] Fix decimal type display issue for record key field

[GitBox <gi...@apache.org>]

umehrot2 commented on a change in pull request #1953:
URL: https://github.com/apache/hudi/pull/1953#discussion_r475021802



##########
File path: hudi-common/src/main/java/org/apache/hudi/avro/HoodieAvroUtils.java
##########
@@ -431,25 +435,30 @@ private static Object convertValueForSpecificDataTypes(Schema fieldSchema, Objec
       return fieldValue;
     }
 
-    if (isLogicalTypeDate(fieldSchema)) {
+    if (fieldSchema.getType() == Schema.Type.UNION) {
+      for (Schema schema : fieldSchema.getTypes()) {
+        if (schema.getLogicalType() == LogicalTypes.date() || schema.getLogicalType() instanceof LogicalTypes.Decimal) {
+          return convertValueForSpecificDataTypes(schema, fieldValue);
+        }
+      }
+    } else if (fieldSchema.getLogicalType() == LogicalTypes.date()) {
+      // special handle for Logical Date type
       return LocalDate.ofEpochDay(Long.parseLong(fieldValue.toString()));
+    } else if (fieldSchema.getLogicalType() instanceof LogicalTypes.Decimal) {
+      // special handle for Logical Decimal type
+      Decimal dc = (Decimal) fieldSchema.getLogicalType();
+      DecimalConversion decimalConversion = new DecimalConversion();
+      if (fieldSchema.getType() == Schema.Type.FIXED) {
+        return decimalConversion.fromFixed((GenericFixed) fieldValue, fieldSchema,
+            LogicalTypes.decimal(dc.getPrecision(), dc.getScale()));
+      } else if (fieldSchema.getType() == Schema.Type.BYTES) {
+        return decimalConversion.fromBytes((ByteBuffer) fieldValue, fieldSchema,
+            LogicalTypes.decimal(dc.getPrecision(), dc.getScale()));
+      }

Review comment:
       
   
   I would suggest to abstract this out into its own function like convertValueForAvroLogicalTypes() and call it from both places instead of using recursive call which is adding overhead of more repetitive checks happening. Since this will be called for each record I would like to keep it as lightweight as possible.
   ```
        if (fieldSchema.getType() == Schema.Type.UNION) {
              for (Schema schema : fieldSchema.getTypes()) {
                     return convertValueForAvroLogicalTypes()
                  }
       } else {
          return convertValueForAvroLogicalTypes()
      }
   ```




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org