yihua commented on code in PR #13711:
URL: https://github.com/apache/hudi/pull/13711#discussion_r2302229412
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieIndexVersion.java:
##########
@@ -85,27 +85,36 @@ public static HoodieIndexVersion
getCurrentVersion(HoodieTableVersion tableVersi
* @return the appropriate HoodieIndexVersion for the given parameters
*/
public static HoodieIndexVersion getCurrentVersion(HoodieTableVersion
tableVersion, MetadataPartitionType partitionType) {
- if (partitionType == MetadataPartitionType.RECORD_INDEX) {
- return V1;
- } else if (partitionType == MetadataPartitionType.COLUMN_STATS) {
- return V1;
- } else if (partitionType == MetadataPartitionType.BLOOM_FILTERS) {
- return V1;
- } else if (partitionType == MetadataPartitionType.EXPRESSION_INDEX) {
- return V1;
- } else if (partitionType == MetadataPartitionType.SECONDARY_INDEX) {
- if (tableVersion.greaterThanOrEquals(HoodieTableVersion.NINE)) {
+ switch (partitionType) {
+ case RECORD_INDEX:
+ return V1;
+
+ case COLUMN_STATS:
+ case PARTITION_STATS:
+ case EXPRESSION_INDEX:
+ // column stats, partition stats, expression index must be updated
together
+ if (tableVersion.lesserThan(HoodieTableVersion.NINE)) {
+ return V1;
+ }
return V2;
- }
- return V1;
- } else if (partitionType == MetadataPartitionType.FILES) {
- return V1;
- } else if (partitionType == MetadataPartitionType.PARTITION_STATS) {
- return V1;
- } else if (partitionType == MetadataPartitionType.ALL_PARTITIONS) {
- return V1;
- } else {
- throw new HoodieException("Unknown metadata partition type: " +
partitionType);
+
+ case BLOOM_FILTERS:
+ return V1;
+
+ case SECONDARY_INDEX:
+ if (tableVersion.greaterThanOrEquals(HoodieTableVersion.NINE)) {
+ return V2;
+ }
+ return V1;
+
+ case FILES:
+ return V1;
+
+ case ALL_PARTITIONS:
+ return V1;
+
+ default:
+ throw new HoodieException("Unknown metadata partition type: " +
partitionType);
Review Comment:
Could we add index version to `MetadataPartitionType` instead of having the
util method here?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieIndexVersion.java:
##########
@@ -85,27 +85,36 @@ public static HoodieIndexVersion
getCurrentVersion(HoodieTableVersion tableVersi
* @return the appropriate HoodieIndexVersion for the given parameters
*/
public static HoodieIndexVersion getCurrentVersion(HoodieTableVersion
tableVersion, MetadataPartitionType partitionType) {
- if (partitionType == MetadataPartitionType.RECORD_INDEX) {
- return V1;
- } else if (partitionType == MetadataPartitionType.COLUMN_STATS) {
- return V1;
- } else if (partitionType == MetadataPartitionType.BLOOM_FILTERS) {
- return V1;
- } else if (partitionType == MetadataPartitionType.EXPRESSION_INDEX) {
- return V1;
- } else if (partitionType == MetadataPartitionType.SECONDARY_INDEX) {
- if (tableVersion.greaterThanOrEquals(HoodieTableVersion.NINE)) {
+ switch (partitionType) {
+ case RECORD_INDEX:
+ return V1;
+
+ case COLUMN_STATS:
+ case PARTITION_STATS:
+ case EXPRESSION_INDEX:
+ // column stats, partition stats, expression index must be updated
together
+ if (tableVersion.lesserThan(HoodieTableVersion.NINE)) {
+ return V1;
+ }
return V2;
- }
- return V1;
- } else if (partitionType == MetadataPartitionType.FILES) {
- return V1;
- } else if (partitionType == MetadataPartitionType.PARTITION_STATS) {
- return V1;
- } else if (partitionType == MetadataPartitionType.ALL_PARTITIONS) {
- return V1;
- } else {
- throw new HoodieException("Unknown metadata partition type: " +
partitionType);
+
+ case BLOOM_FILTERS:
+ return V1;
+
+ case SECONDARY_INDEX:
+ if (tableVersion.greaterThanOrEquals(HoodieTableVersion.NINE)) {
+ return V2;
+ }
+ return V1;
+
+ case FILES:
+ return V1;
+
+ case ALL_PARTITIONS:
+ return V1;
+
+ default:
+ throw new HoodieException("Unknown metadata partition type: " +
partitionType);
Review Comment:
The problem of the current approach is that, if a new metadata partition
type is added, both `HoodieIndexVersion` and `MetadataPartitionType` have to be
updated. Instead, a developer should only update `MetadataPartitionType` for
basic info for new partition type.
--
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]
