satishkotha commented on a change in pull request #1320: [HUDI-571] Add min/max 
headers on archived files
URL: https://github.com/apache/incubator-hudi/pull/1320#discussion_r381034791
 
 

 ##########
 File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/log/block/HoodieLogBlock.java
 ##########
 @@ -121,7 +121,7 @@ public long getLogBlockLength() {
    * new enums at the end.
    */
   public enum HeaderMetadataType {
-    INSTANT_TIME, TARGET_INSTANT_TIME, SCHEMA, COMMAND_BLOCK_TYPE
+    INSTANT_TIME, TARGET_INSTANT_TIME, SCHEMA, COMMAND_BLOCK_TYPE, 
MIN_INSTANT_TIME, MAX_INSTANT_TIME
 
 Review comment:
   I tried doing this. there are couple problems:
   1) java types- HoodieAvroDataBlock relies on header key to be of type: 
HeaderMetadataType. Nested enum has different java type, so this requires lot 
more refactoring. enums do not seem to have inheritance support in java, so 
this likely involves converting enum to string/ordinals and lot of testing to 
make sure we did not break any backward compatibility
   
   2) enum nesting will end up creating totally new set of ordinals. It can be 
confusing to see which enum ordinals show. 
   
   tl;dr building better abstraction seems lot more work. Let me know if we 
want to investigate this further. Likely will have to done as separate task and 
postpone/abandon merging this until then.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to