yihua commented on code in PR #13523:
URL: https://github.com/apache/hudi/pull/13523#discussion_r2229159780
##########
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:
Let's avoid refactoring among major changes to reduce review overhead in the
future.
##########
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
-> {
Review Comment:
How is this different from `lookupRecordsItr`? Any reason to keep both
methods, one returning `HoodieRecord<HoodieMetadataPayload>` and the other
returning `Pair<String, HoodieRecord<HoodieMetadataPayload>>`?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -313,14 +293,13 @@ private HoodiePairData<String,
HoodieRecord<HoodieMetadataPayload>> doLookup(Hoo
distinctSortedKeyIter.forEachRemaining(keysList::add);
}
FileSlice fileSlice =
fileSlices.get(mappingFunction.apply(keysList.get(0), numFileSlices));
- return lookupKeyRecordPairsItr(partitionName, keysList, fileSlice,
!isSecondaryIndex);
+ return lookupKeyRecordPairsItr(partitionName, keysList, fileSlice);
Review Comment:
The naming should be better. Let's tackle it separate to avoid merge
conflict.
- `lookupKeyRecordPairs` -> `lookUpSortedUniqueKeys`
- `lookupKeyRecordPairsItr` -> `lookUpSortedUniqueKeysIterator`
##########
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) {
Review Comment:
rename the method to be clear: `lookupRecordsItr` -> `lookUpFullKeys`
--
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]
