[GitHub] [hudi] boneanxs commented on a diff in pull request #6793: 【HUDI-4917】Optimized the way to get HoodieBaseFile of loadColumnRange…

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

boneanxs commented on code in PR #6793:
URL: https://github.com/apache/hudi/pull/6793#discussion_r984153227


##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/index/bloom/HoodieBloomIndex.java:
##########
@@ -161,19 +162,19 @@ private List<Pair<String, BloomIndexFileInfo>> getBloomIndexFileInfoForPartition
   List<Pair<String, BloomIndexFileInfo>> loadColumnRangesFromFiles(
       List<String> partitions, final HoodieEngineContext context, final HoodieTable hoodieTable) {
     // Obtain the latest data files from all the partitions.
-    List<Pair<String, String>> partitionPathFileIDList = getLatestBaseFilesForAllPartitions(partitions, context, hoodieTable).stream()
-        .map(pair -> Pair.of(pair.getKey(), pair.getValue().getFileId()))
+    List<Pair<String, Pair<String, HoodieBaseFile>>> partitionPathFileIDList = getLatestBaseFilesForAllPartitions(partitions, context, hoodieTable).stream()
+        .map(pair -> Pair.of(pair.getKey(), Pair.of(pair.getValue().getFileId(), pair.getValue())))
         .collect(toList());
 
     context.setJobStatus(this.getClass().getName(), "Obtain key ranges for file slices (range pruning=on): " + config.getTableName());
     return context.map(partitionPathFileIDList, pf -> {
       try {
-        HoodieRangeInfoHandle rangeInfoHandle = new HoodieRangeInfoHandle(config, hoodieTable, pf);
-        String[] minMaxKeys = rangeInfoHandle.getMinMaxKeys();
-        return Pair.of(pf.getKey(), new BloomIndexFileInfo(pf.getValue(), minMaxKeys[0], minMaxKeys[1]));
+        HoodieRangeInfoHandle rangeInfoHandle = new HoodieRangeInfoHandle(config, hoodieTable, Pair.of(pf.getKey(), pf.getValue().getKey()));
+        String[] minMaxKeys = rangeInfoHandle.getMinMaxKeys(pf.getValue().getValue());

Review Comment:
   I think `HoodieRangeInfoHandle` is bind to a file slice, but here you break the class meaning to allow it handle different files. Maybe we can change the class construct to accept `BaseFile`, while keep the method as it is before.



-- 
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