Davis-Zhang-Onehouse commented on code in PR #13523:
URL: https://github.com/apache/hudi/pull/13523#discussion_r2229419472


##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -631,6 +603,28 @@ private <T> ClosableIterator<T> lookupRecords(List<String> 
sortedKeys,
     }
   }
 
+  static Predicate buildPredicate(String partitionName, List<String> 
sortedKeys, boolean isFullKey) {
+    if (MetadataPartitionType.fromPartitionPath(partitionName)
+          .equals(MetadataPartitionType.SECONDARY_INDEX)) {
+      // For secondary index, always use prefix matching

Review Comment:
   added doc



##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -563,63 +536,62 @@ private HoodieFileGroupReader<IndexedRecord> 
buildFileGroupReader(List<String> s
 
   private HoodiePairData<String, HoodieRecord<HoodieMetadataPayload>> 
lookupKeyRecordPairs(String partitionName,
                                                                                
            List<String> sortedKeys,
-                                                                               
            FileSlice fileSlice,
-                                                                               
            Boolean isFullKey) {
+                                                                               
            FileSlice fileSlice) {
     Map<String, List<HoodieRecord<HoodieMetadataPayload>>> map = new 
HashMap<>();
     try (ClosableIterator<Pair<String, HoodieRecord<HoodieMetadataPayload>>> 
iterator =
-             lookupKeyRecordPairsItr(partitionName, sortedKeys, fileSlice, 
isFullKey)) {
+             lookupKeyRecordPairsItr(partitionName, sortedKeys, fileSlice)) {
       iterator.forEachRemaining(entry -> map.put(entry.getKey(), 
Collections.singletonList(entry.getValue())));
     }
     return HoodieListPairData.eager(map);
   }
 
   private HoodieData<HoodieRecord<HoodieMetadataPayload>> lookupRecords(String 
partitionName,
                                                                         
List<String> sortedKeys,
-                                                                        
FileSlice fileSlice,
-                                                                        
Boolean isFullKey) {
+                                                                        
FileSlice fileSlice) {
     List<HoodieRecord<HoodieMetadataPayload>> res = new ArrayList<>();
-    try (ClosableIterator<HoodieRecord<HoodieMetadataPayload>> iterator = 
lookupRecordsItr(partitionName, sortedKeys, fileSlice, isFullKey)) {
+    try (ClosableIterator<HoodieRecord<HoodieMetadataPayload>> iterator = 
lookupRecordsItr(partitionName, sortedKeys, fileSlice)) {
       iterator.forEachRemaining(res::add);
     }
     return HoodieListData.lazy(res);
   }
 
   private ClosableIterator<Pair<String, HoodieRecord<HoodieMetadataPayload>>> 
lookupKeyRecordPairsItr(String partitionName,
                                                                                
                       List<String> sortedKeys,
-                                                                               
                       FileSlice fileSlice,
-                                                                               
                       Boolean isFullKey) {
-    return lookupRecords(sortedKeys, fileSlice, isFullKey, metadataRecord -> {
+                                                                               
                       FileSlice fileSlice) {
+    return lookupRecords(partitionName, sortedKeys, fileSlice, metadataRecord 
-> {
       HoodieMetadataPayload payload = new 
HoodieMetadataPayload(Option.of(metadataRecord));
       String rowKey = payload.key != null ? payload.key : 
metadataRecord.get(KEY_FIELD_NAME).toString();
       HoodieKey hoodieKey = new HoodieKey(rowKey, partitionName);
       return Pair.of(rowKey, new HoodieAvroRecord<>(hoodieKey, payload));
-    });
+    }, true);
   }
 
   private ClosableIterator<HoodieRecord<HoodieMetadataPayload>> 
lookupRecordsItr(String partitionName,
                                                                                
  List<String> keys,
-                                                                               
  FileSlice fileSlice,
-                                                                               
  Boolean isFullKey) {
-    return lookupRecords(keys, fileSlice, isFullKey,
+                                                                               
  FileSlice fileSlice) {
+    return lookupRecords(partitionName, keys, fileSlice,
         metadataRecord -> {
           HoodieMetadataPayload payload = new 
HoodieMetadataPayload(Option.of(metadataRecord));
           return new HoodieAvroRecord<>(new HoodieKey(payload.key, 
partitionName), payload);
-        });
+        }, true);
   }
 
   /**
    * Lookup records and produce a lazy iterator of mapped HoodieRecords.
+   * @param isFullKey If true, perform exact key match. If false, perform 
prefix match.
    */
-  private <T> ClosableIterator<T> lookupRecords(List<String> sortedKeys,
+  private <T> ClosableIterator<T> lookupRecords(String partitionName,
+                                                List<String> sortedKeys,
                                                 FileSlice fileSlice,
-                                                Boolean isFullKey,
-                                                
SerializableFunctionUnchecked<GenericRecord, T> transformer) {
+                                                
SerializableFunctionUnchecked<GenericRecord, T> transformer,
+                                                boolean isFullKey) {

Review Comment:
   acked



##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -631,6 +603,28 @@ private <T> ClosableIterator<T> lookupRecords(List<String> 
sortedKeys,
     }
   }
 
+  static Predicate buildPredicate(String partitionName, List<String> 
sortedKeys, boolean isFullKey) {
+    if (MetadataPartitionType.fromPartitionPath(partitionName)
+          .equals(MetadataPartitionType.SECONDARY_INDEX)) {
+      // For secondary index, always use prefix matching
+      return Predicates.startsWithAny(null,
+          sortedKeys.stream()
+          .map(SecondaryIndexKeyUtils::constructSecondaryIndexKeyPrefix)

Review Comment:
   moved the func to the consumer class itself.



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

Reply via email to