yihua commented on code in PR #12829:
URL: https://github.com/apache/hudi/pull/12829#discussion_r1970604459
##########
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:
I assume this class is still need to commit metadata SerDe V1 which does not
leverage the Avro schema, correct?
##########
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:
```suggestion
/**
* Convert replacecommit metadata from POJO to Avro.
*/
private static org.apache.hudi.avro.model.HoodieReplaceCommitMetadata
convertReplaceCommitMetadataToAvro(HoodieReplaceCommitMetadata
replaceCommitMetadata) {
return JsonUtils.getObjectMapper().convertValue(replaceCommitMetadata,
org.apache.hudi.avro.model.HoodieReplaceCommitMetadata.class);
}
```
##########
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:
If we could use Avro schema for JSON SerDe as well, it's better to remove
this class completely.
##########
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:
```suggestion
/**
* Convert replacecommit metadata from Avro to POJO.
*/
public static HoodieReplaceCommitMetadata
convertReplaceCommitMetadataToPojo(org.apache.hudi.avro.model.HoodieReplaceCommitMetadata
replaceCommitMetadata) {
```
--
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]
