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