alexeykudinkin commented on code in PR #7744:
URL: https://github.com/apache/hudi/pull/7744#discussion_r1085907323
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieTableMetadata.java:
##########
@@ -138,12 +138,15 @@ static HoodieBackedTableMetadata
createHoodieBackedTableMetadata(HoodieEngineCon
FileStatus[] getAllFilesInPartition(Path partitionPath) throws IOException;
/**
- * Fetch list of all partitions path that whose relative partition paths
match the given prefixes
+ * Fetch list of all partitions path that whose relative partition paths is
under the
+ * directories based on the given relative paths.
+ * <p>
* E.g., Table has partition 4 partitions:
- * year=2022/month=08/day=30, year=2022/month=08/day=31,
year=2022/month=07/day=03, year=2022/month=07/day=04
- * Prefix "year=2022" will return all partitions, while prefix
"year=2022/month=07" will output only two partitions.
+ * year=2022/month=08/day=30, year=2022/month=08/day=31,
year=2022/month=07/day=03, year=2022/month=07/day=04
+ * The relative path "year=2022" returns all partitions, while the relative
path
Review Comment:
👍
##########
hudi-common/src/main/java/org/apache/hudi/metadata/FileSystemBackedTableMetadata.java:
##########
@@ -81,23 +81,23 @@ public List<String> getAllPartitionPaths() throws
IOException {
return FSUtils.getAllPartitionFoldersThreeLevelsDown(fs,
datasetBasePath);
}
- return getPartitionPathsWithPrefixes(Collections.singletonList(""));
+ return getPartitionPathsInDirs(Collections.singletonList(""));
}
@Override
- public List<String> getPartitionPathsWithPrefixes(List<String> prefixes) {
- return prefixes.stream().flatMap(prefix -> {
+ public List<String> getPartitionPathsInDirs(List<String> relativePaths) {
Review Comment:
While i'm not a huge fan of `getPartitionPathsWithPrefix` name, i find
`getPartitionPathsInDirs` more confusing than the previous one (it's not clear
on why we're looking up partition-paths in some directories)
I think we should instead clarify the prefix part of this naming to be
proper _path_ prefix, like for ex `getPartitionPathWithPathPrefix` and make a
clear call out in the java-doc. WDYT?
##########
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadata.java:
##########
@@ -150,9 +152,11 @@ protected Option<HoodieRecord<HoodieMetadataPayload>>
getRecordByKey(String key,
}
@Override
- public List<String> getPartitionPathsWithPrefixes(List<String> prefixes)
throws IOException {
+ public List<String> getPartitionPathsInDirs(List<String> relativePaths)
throws IOException {
return getAllPartitionPaths().stream()
- .filter(p -> prefixes.stream().anyMatch(p::startsWith))
+ .filter(p -> relativePaths.stream().anyMatch(relativePath ->
+ StringUtils.isNullOrEmpty(relativePath)
+ || p.equals(relativePath) || p.startsWith(relativePath + "/")))
Review Comment:
Let's add a comment to elaborate on the semantic of our intent here
--
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]
