Davis-Zhang-Onehouse commented on code in PR #12829: URL: https://github.com/apache/hudi/pull/12829#discussion_r1970659835
########## hudi-common/src/main/java/org/apache/hudi/common/model/HoodieCommitMetadata.java: ########## @@ -45,6 +45,13 @@ /** * All the metadata that gets stored along with a commit. + * ******** IMPORTANT ******** + * For any newly added/removed data fields, make sure we have the same definition in + * src/main/avro/HoodieCommitMetadata.avsc file!!!!! + * + * For any newly added subclass, make sure we add corresponding handler in + * org.apache.hudi.common.table.timeline.versioning.v2.CommitMetadataSerDeV2#deserialize method. + * *************************** Review Comment: https://issues.apache.org/jira/browse/HUDI-9078 ########## hudi-common/src/main/java/org/apache/hudi/common/table/timeline/MetadataConversionUtils.java: ########## @@ -364,18 +364,32 @@ public static <T extends SpecificRecordBase> T convertCommitMetadata(HoodieCommi if (hoodieCommitMetadata instanceof HoodieReplaceCommitMetadata) { return (T) convertReplaceCommitMetadata((HoodieReplaceCommitMetadata) hoodieCommitMetadata); } - hoodieCommitMetadata.getPartitionToWriteStats().remove(null); org.apache.hudi.avro.model.HoodieCommitMetadata avroMetaData = JsonUtils.getObjectMapper().convertValue(hoodieCommitMetadata, org.apache.hudi.avro.model.HoodieCommitMetadata.class); return (T) avroMetaData; } + /** + * Convert commit metadata from json to avro. + */ + public static HoodieCommitMetadata convertCommitMetadataAvroToPojo(org.apache.hudi.avro.model.HoodieCommitMetadata hoodieCommitMetadata) { + hoodieCommitMetadata.getPartitionToWriteStats().remove(null); Review Comment: done. also fixed the java doc ########## hudi-common/src/main/java/org/apache/hudi/common/table/timeline/TimelineMetadataUtils.java: ########## @@ -204,7 +204,7 @@ public static HoodieReplaceCommitMetadata deserializeReplaceCommitMetadata(byte[ return deserializeAvroMetadata(bytes, HoodieReplaceCommitMetadata.class); } - public static <T extends SpecificRecordBase> T deserializeAvroMetadata(byte[] bytes, Class<T> clazz) + public static <T> T deserializeAvroMetadata(byte[] bytes, Class<T> clazz) Review Comment: reverted ########## hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieCommitMetadata.java: ########## @@ -190,6 +196,187 @@ private org.apache.hudi.avro.model.HoodieWriteStat createWriteStat(String fileId writeStat.setFileId(fileId); writeStat.setBaseFile(baseFile); writeStat.setLogFiles(logFiles); - return writeStat; + return writeStat; + } + + @Test + public void testSchemaEqualityForHoodieCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); + + // Fail if there are any critical errors (field missing or type mismatch at current layer) + assertFalse(result.hasCriticalErrors(), "Critical validation errors found"); + } + + @Test + public void testSchemaEqualityForHoodieReplaceCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieReplaceCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieReplaceCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); Review Comment: removed ########## hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieCommitMetadata.java: ########## @@ -190,6 +196,187 @@ private org.apache.hudi.avro.model.HoodieWriteStat createWriteStat(String fileId writeStat.setFileId(fileId); writeStat.setBaseFile(baseFile); writeStat.setLogFiles(logFiles); - return writeStat; + return writeStat; + } + + @Test + public void testSchemaEqualityForHoodieCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); + + // Fail if there are any critical errors (field missing or type mismatch at current layer) + assertFalse(result.hasCriticalErrors(), "Critical validation errors found"); + } + + @Test + public void testSchemaEqualityForHoodieReplaceCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieReplaceCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieReplaceCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); + + // Fail if there are any critical errors (field missing or type mismatch at current layer) + assertFalse(result.hasCriticalErrors(), "Critical validation errors found"); + } + + private static class ValidationResult { + boolean hasCriticalErrors = false; + List<String> errors = new ArrayList<>(); + List<ValidationResult> nestedResults = new ArrayList<>(); + + void addError(String error) { + errors.add(error); + } + + void addNestedResult(ValidationResult result) { + nestedResults.add(result); + } + + void markCritical() { + hasCriticalErrors = true; + } + + boolean hasCriticalErrors() { + return hasCriticalErrors; + } + } + + private void printValidationResults(ValidationResult result) { + printValidationResults(result, 0); + } + + private void printValidationResults(ValidationResult result, int indent) { + // We don't have repeat method in java 8 + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < indent; i++) { + sb.append(" "); + } + for (String error : result.errors) { + LOGGER.error(sb + error); + } + for (ValidationResult nested : result.nestedResults) { + printValidationResults(nested, indent + 1); + } + } + + private ValidationResult validateSchemas(Schema pojoSchema, Schema avroSchema, String path) { Review Comment: done ########## hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieCommitMetadata.java: ########## @@ -190,6 +196,187 @@ private org.apache.hudi.avro.model.HoodieWriteStat createWriteStat(String fileId writeStat.setFileId(fileId); writeStat.setBaseFile(baseFile); writeStat.setLogFiles(logFiles); - return writeStat; + return writeStat; + } + + @Test + public void testSchemaEqualityForHoodieCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); Review Comment: removed ########## hudi-common/src/main/java/org/apache/hudi/common/table/timeline/MetadataConversionUtils.java: ########## @@ -364,18 +364,32 @@ public static <T extends SpecificRecordBase> T convertCommitMetadata(HoodieCommi if (hoodieCommitMetadata instanceof HoodieReplaceCommitMetadata) { return (T) convertReplaceCommitMetadata((HoodieReplaceCommitMetadata) hoodieCommitMetadata); } - hoodieCommitMetadata.getPartitionToWriteStats().remove(null); org.apache.hudi.avro.model.HoodieCommitMetadata avroMetaData = JsonUtils.getObjectMapper().convertValue(hoodieCommitMetadata, org.apache.hudi.avro.model.HoodieCommitMetadata.class); return (T) avroMetaData; } + /** + * Convert commit metadata from json to avro. + */ + public static HoodieCommitMetadata convertCommitMetadataAvroToPojo(org.apache.hudi.avro.model.HoodieCommitMetadata hoodieCommitMetadata) { + hoodieCommitMetadata.getPartitionToWriteStats().remove(null); + return JsonUtils.getObjectMapper().convertValue(hoodieCommitMetadata, HoodieCommitMetadata.class); + } + /** * Convert replacecommit metadata from json to avro. */ private static org.apache.hudi.avro.model.HoodieReplaceCommitMetadata convertReplaceCommitMetadata(HoodieReplaceCommitMetadata replaceCommitMetadata) { + return JsonUtils.getObjectMapper().convertValue(replaceCommitMetadata, org.apache.hudi.avro.model.HoodieReplaceCommitMetadata.class); + } Review Comment: done ########## hudi-common/src/main/java/org/apache/hudi/common/table/timeline/MetadataConversionUtils.java: ########## @@ -364,18 +364,32 @@ public static <T extends SpecificRecordBase> T convertCommitMetadata(HoodieCommi if (hoodieCommitMetadata instanceof HoodieReplaceCommitMetadata) { return (T) convertReplaceCommitMetadata((HoodieReplaceCommitMetadata) hoodieCommitMetadata); } - hoodieCommitMetadata.getPartitionToWriteStats().remove(null); org.apache.hudi.avro.model.HoodieCommitMetadata avroMetaData = JsonUtils.getObjectMapper().convertValue(hoodieCommitMetadata, org.apache.hudi.avro.model.HoodieCommitMetadata.class); return (T) avroMetaData; } + /** + * Convert commit metadata from json to avro. + */ + public static HoodieCommitMetadata convertCommitMetadataAvroToPojo(org.apache.hudi.avro.model.HoodieCommitMetadata hoodieCommitMetadata) { + hoodieCommitMetadata.getPartitionToWriteStats().remove(null); + return JsonUtils.getObjectMapper().convertValue(hoodieCommitMetadata, HoodieCommitMetadata.class); + } + /** * Convert replacecommit metadata from json to avro. */ private static org.apache.hudi.avro.model.HoodieReplaceCommitMetadata convertReplaceCommitMetadata(HoodieReplaceCommitMetadata replaceCommitMetadata) { + return JsonUtils.getObjectMapper().convertValue(replaceCommitMetadata, org.apache.hudi.avro.model.HoodieReplaceCommitMetadata.class); + } + + /** + * Convert replacecommit metadata from json to avro. + */ + public static HoodieReplaceCommitMetadata convertReplaceCommitMetadataAvroToPojo(org.apache.hudi.avro.model.HoodieReplaceCommitMetadata replaceCommitMetadata) { Review Comment: done ########## hudi-common/src/test/java/org/apache/hudi/common/model/TestHoodieCommitMetadata.java: ########## @@ -190,6 +196,187 @@ private org.apache.hudi.avro.model.HoodieWriteStat createWriteStat(String fileId writeStat.setFileId(fileId); writeStat.setBaseFile(baseFile); writeStat.setLogFiles(logFiles); - return writeStat; + return writeStat; + } + + @Test + public void testSchemaEqualityForHoodieCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); + + // Fail if there are any critical errors (field missing or type mismatch at current layer) + assertFalse(result.hasCriticalErrors(), "Critical validation errors found"); + } + + @Test + public void testSchemaEqualityForHoodieReplaceCommitMetaData() { + // Step 1: Get the schema from the Avro auto-generated class + Schema avroSchema = org.apache.hudi.avro.model.HoodieReplaceCommitMetadata.SCHEMA$; + + // Step 2: Convert the POJO class to an Avro schema + Schema pojoSchema = ReflectData.get().getSchema(org.apache.hudi.common.model.HoodieReplaceCommitMetadata.class); + + // Step 3: Validate schemas + ValidationResult result = validateSchemas(pojoSchema, avroSchema, "root"); + + // Print validation results + printValidationResults(result); + + // Fail if there are any critical errors (field missing or type mismatch at current layer) + assertFalse(result.hasCriticalErrors(), "Critical validation errors found"); + } + + private static class ValidationResult { + boolean hasCriticalErrors = false; + List<String> errors = new ArrayList<>(); + List<ValidationResult> nestedResults = new ArrayList<>(); + + void addError(String error) { + errors.add(error); + } + + void addNestedResult(ValidationResult result) { + nestedResults.add(result); + } + + void markCritical() { + hasCriticalErrors = true; + } + + boolean hasCriticalErrors() { + return hasCriticalErrors; + } + } + + private void printValidationResults(ValidationResult result) { + printValidationResults(result, 0); + } + + private void printValidationResults(ValidationResult result, int indent) { + // We don't have repeat method in java 8 + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < indent; i++) { + sb.append(" "); + } + for (String error : result.errors) { + LOGGER.error(sb + error); + } + for (ValidationResult nested : result.nestedResults) { + printValidationResults(nested, indent + 1); + } + } Review Comment: removed -- 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]
