[GitHub] [hudi] xushiyan commented on a change in pull request #4811: [HUDI-3213] Making commit preserve metadata to true for compaction

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

xushiyan commented on a change in pull request #4811:
URL: https://github.com/apache/hudi/pull/4811#discussion_r820718250



##########
File path: hudi-client/hudi-client-common/src/main/java/org/apache/hudi/io/HoodieMergeHandle.java
##########
@@ -292,8 +294,10 @@ protected boolean writeRecord(HoodieRecord<T> hoodieRecord, Option<IndexedRecord
     try {
       if (indexedRecord.isPresent() && !isDelete) {
         // Convert GenericRecord to GenericRecord with hoodie commit metadata in schema
-        IndexedRecord recordWithMetadataInSchema = rewriteRecord((GenericRecord) indexedRecord.get());
-        if (preserveMetadata) {
+        IndexedRecord recordWithMetadataInSchema = rewriteRecord((GenericRecord) indexedRecord.get(), preserveMetadata, oldRecord);
+        if (preserveMetadata && useWriterSchema) { // useWriteSchema will be true only incase of compaction.

Review comment:
       if this is the case, better name could be `useWriterSchemaForCompaction`

##########
File path: hudi-common/src/main/java/org/apache/hudi/common/model/HoodieRecord.java
##########
@@ -42,6 +42,8 @@
   public static final String OPERATION_METADATA_FIELD = "_hoodie_operation";
   public static final String HOODIE_IS_DELETED = "_hoodie_is_deleted";
 
+  public static int FILENAME_METADATA_FIELD_POS = 4;

Review comment:
       this could be a separate cleanup task: make constants for all meta fields and adopt them across codebase

##########
File path: hudi-common/src/main/java/org/apache/hudi/avro/HoodieAvroUtils.java
##########
@@ -382,10 +382,34 @@ public static GenericRecord rewriteRecord(GenericRecord oldRecord, Schema newSch
     return newRecord;
   }
 
+  public static GenericRecord rewriteRecord(GenericRecord genericRecord, Schema newSchema, boolean copyOverMetaFields, GenericRecord fallbackRecord) {

Review comment:
       would be good to have at least 1 UT covering this 




-- 
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: commits-unsubscribe@hudi.apache.org

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