nsivabalan commented on code in PR #17859:
URL: https://github.com/apache/hudi/pull/17859#discussion_r2720199429
##########
hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java:
##########
@@ -314,6 +314,17 @@ public static String createNewFileId(String idPfx, int id)
{
.toString();
}
+ public static StoragePath getPartitionPath(StoragePath basePath, String
partition) {
+ return StringUtils.isNullOrEmpty(partition)
+ ? basePath : new StoragePath(basePath, partition);
+ }
+
+ public static StoragePath getFilePath(StoragePath basePath, String
partition, String fileName) {
Review Comment:
getAbsoluteFilePath
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java:
##########
@@ -723,6 +723,16 @@ public HoodieTestTable withPartitionMetaFiles(List<String>
partitionPaths) throw
return this;
}
+ public HoodieTestTable withPartitionMetaFilesWithDepth(String...
partitionPaths) throws IOException {
+ for (String partitionPath : partitionPaths) {
+ StoragePath partitionPathObj = FSUtils.getPartitionPath(new
StoragePath(basePath), partitionPath);
+ HoodiePartitionMetadata partitionMetadata1 = new
HoodiePartitionMetadata(storage,
+ currentInstantTime, new StoragePath(basePath), partitionPathObj,
Option.of(HoodieFileFormat.PARQUET));
Review Comment:
last arg is optional right.
why explicitly set it to `HoodieFileFormat.PARQUET` ?
we can leave it empty right.
##########
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieROTablePathFilter.java:
##########
@@ -192,10 +216,10 @@ public boolean accept(Path path) {
// which contains old version files, if not specify this value,
these files will be filtered.
fsView =
FileSystemViewManager.createInMemoryFileSystemViewWithTimeline(engineContext,
metaClient, HoodieInputFormatUtils.buildMetadataConfig(conf),
-
metaClient.getActiveTimeline().filterCompletedInstants().findInstantsBeforeOrEquals(timestampAsOf));
+ completedTimeline.findInstantsBeforeOrEquals(timestampAsOf));
} else {
- fsView =
FileSystemViewManager.createInMemoryFileSystemView(engineContext,
Review Comment:
w/n FileSystemViewManager.createInMemoryFileSystemView, we actually look for
`commitsTimeline`
```
metaClient.getActiveTimeline().getCommitsTimeline().filterCompletedInstants()
```
but w/ the caching, we are fetching entire active timeline and then filter
for completed ( L206).
##########
hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java:
##########
@@ -314,6 +314,17 @@ public static String createNewFileId(String idPfx, int id)
{
.toString();
}
+ public static StoragePath getPartitionPath(StoragePath basePath, String
partition) {
Review Comment:
minor : `getAbsolutePartitionPath`
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java:
##########
@@ -723,6 +723,16 @@ public HoodieTestTable withPartitionMetaFiles(List<String>
partitionPaths) throw
return this;
}
+ public HoodieTestTable withPartitionMetaFilesWithDepth(String...
partitionPaths) throws IOException {
+ for (String partitionPath : partitionPaths) {
+ StoragePath partitionPathObj = FSUtils.getPartitionPath(new
StoragePath(basePath), partitionPath);
+ HoodiePartitionMetadata partitionMetadata1 = new
HoodiePartitionMetadata(storage,
Review Comment:
why `partitionMetadata1` and not `partitionMetadata` ?
##########
hudi-hadoop-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java:
##########
@@ -723,6 +723,16 @@ public HoodieTestTable withPartitionMetaFiles(List<String>
partitionPaths) throw
return this;
}
+ public HoodieTestTable withPartitionMetaFilesWithDepth(String...
partitionPaths) throws IOException {
Review Comment:
minor. we could name this method `addPartitionMetaFiles`
--
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]
