Davis-Zhang-Onehouse commented on code in PR #11265:
URL: https://github.com/apache/hudi/pull/11265#discussion_r1690309755


##########
hudi-common/src/test/java/org/apache/hudi/avro/TestMercifulJsonConverter.java:
##########
@@ -55,6 +70,649 @@ public void basicConversion() throws IOException {
     Assertions.assertEquals(rec, CONVERTER.convert(json, simpleSchema));
   }
 
+  private static final String DECIMAL_AVRO_FILE_INVALID_PATH = 
"/decimal-logical-type-invalid.avsc";
+  private static final String DECIMAL_AVRO_FILE_PATH = 
"/decimal-logical-type.avsc";
+  private static final String DECIMAL_FIXED_AVRO_FILE_PATH = 
"/decimal-logical-type-fixed-type.avsc";
+  /**
+   * Covered case:
+   * Avro Logical Type: Decimal
+   * Exhaustive unsupported input coverage.
+   */
+  @ParameterizedTest
+  @MethodSource("decimalBadCases")
+  void decimalLogicalTypeInvalidCaseTest(String avroFile, String strInput, 
Double numInput,
+                                         boolean testFixedByteArray) throws 
IOException {
+    Schema schema = SchemaTestUtil.getSchemaFromResourceFilePath(avroFile);
+
+    Map<String, Object> data = new HashMap<>();
+    if (strInput != null) {
+      data.put("decimalField", strInput);
+    } else if (numInput != null) {
+      data.put("decimalField", numInput);
+    } else if (testFixedByteArray) {
+      // Convert the fixed value to int array, which is used as json value 
literals.
+      int[] intArray = {0, 0, 48, 57};
+      data.put("decimalField", intArray);
+    }
+    String json = MAPPER.writeValueAsString(data);
+
+    // Schedule with timestamp same as that of committed instant
+    
assertThrows(MercifulJsonConverter.HoodieJsonToAvroConversionException.class, 
() -> {
+      CONVERTER.convert(json, schema);
+    });
+  }
+
+  static Stream<Object> decimalBadCases() {
+    return Stream.of(
+        // Invalid schema definition.
+        Arguments.of(DECIMAL_AVRO_FILE_INVALID_PATH, "123.45", null, false),
+        // Schema set precision as 5, input overwhelmed the precision.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "123333.45", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, null, 123333.45, false),
+        // Schema precision set to 5, scale set to 2, so there is only 3 digit 
to accommodate integer part.
+        // As we do not do rounding, any input with more than 3 digit integer 
would fail.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "1233", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, null, 1233D, false),
+        // Schema set scale as 2, input overwhelmed the scale.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "0.222", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, null, 0.222, false),
+        // Invalid string which cannot be parsed as number.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "NotAValidString", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "-", null, false),
+        // Schema requires byte type while input is fixed type raw data.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, null, null, true)
+    );
+  }
+
+  /**
+   * Covered case:
+   * Avro Logical Type: Decimal
+   * Avro type: bytes, fixed
+   * Input: Check test parameter
+   * Output: Object using Byte data type as the schema specified.
+   * */
+  @ParameterizedTest
+  @MethodSource("decimalGoodCases")
+  void decimalLogicalTypeTest(String avroFilePath, String groundTruth, String 
strInput,
+                              Double numInput, boolean testFixedByteArray) 
throws IOException {
+    BigDecimal bigDecimal = new BigDecimal(groundTruth);
+    Map<String, Object> data = new HashMap<>();
+
+    Schema schema = SchemaTestUtil.getSchemaFromResourceFilePath(avroFilePath);
+    GenericRecord record = new GenericData.Record(schema);
+    Conversions.DecimalConversion conv = new Conversions.DecimalConversion();
+    Schema decimalFieldSchema = schema.getField("decimalField").schema();
+
+    // Decide the decimal field input according to the test dimension.
+    if (strInput != null) {
+      data.put("decimalField", strInput); // String number input
+    } else if (numInput != null) {
+      data.put("decimalField", numInput); // Number input
+    } else if (testFixedByteArray) {
+      // Fixed byte array input.
+      // Example: 123.45 - byte array [0, 0, 48, 57].
+      Schema fieldSchema = schema.getField("decimalField").schema();
+      GenericFixed fixedValue = new Conversions.DecimalConversion().toFixed(
+          bigDecimal, fieldSchema, fieldSchema.getLogicalType());
+      // Convert the fixed value to int array, which is used as json value 
literals.
+      byte[] byteArray = fixedValue.bytes();
+      int[] intArray = new int[byteArray.length];
+      for (int i = 0; i < byteArray.length; i++) {
+        // Byte is signed in Java, int is 32-bit. Convert by & 0xFF to handle 
negative values correctly.
+        intArray[i] = byteArray[i] & 0xFF;
+      }
+      data.put("decimalField", intArray);
+    }
+
+    // Decide the decimal field expected output according to the test 
dimension.
+    if (avroFilePath.equals(DECIMAL_AVRO_FILE_PATH)) {
+      record.put("decimalField", conv.toBytes(bigDecimal, decimalFieldSchema, 
decimalFieldSchema.getLogicalType()));
+    } else {
+      record.put("decimalField", conv.toFixed(bigDecimal, decimalFieldSchema, 
decimalFieldSchema.getLogicalType()));
+    }
+
+    String json = MAPPER.writeValueAsString(data);
+
+    GenericRecord real = CONVERTER.convert(json, schema);
+    Assertions.assertEquals(record, real);
+  }
+
+  static Stream<Object> decimalGoodCases() {
+    return Stream.of(
+        // The schema all set precision as 5, scale as 2.
+        // Test dimension: Schema file, Ground truth, string input, number 
input, fixed byte array input.
+        // Test some random numbers.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "123.45", "123.45", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "123.45", null, 123.45, false),
+        // Test MIN/MAX allowed by the schema.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "-999.99", "-999.99", null, 
false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "999.99",null, 999.99, false),
+        // Test 0.
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "0", null, 0D, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "0", "0", null, false),
+        Arguments.of(DECIMAL_AVRO_FILE_PATH, "0", "000.00", null, false),
+        // Same set of coverage over schame using byte/fixed type.
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "123.45", "123.45", null, 
false),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "123.45", null, 123.45, 
false),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "-999.99", "-999.99", null, 
false),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "999.99",null, 999.99, 
false),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "0", null, 0D, false),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "0", "0", null, true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "0", "000.00", null, true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "123.45", null, null, true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "123.45", null, 123.45, 
true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "-999.99", null, null, 
true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "999.99", null, 999.99, 
true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "0", null, null, true),
+        Arguments.of(DECIMAL_FIXED_AVRO_FILE_PATH, "0", null, null, true),

Review Comment:
   done



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