cshuo commented on code in PR #19198:
URL: https://github.com/apache/hudi/pull/19198#discussion_r3536188389


##########
hudi-common/src/main/java/org/apache/hudi/common/fs/FSUtils.java:
##########
@@ -489,27 +425,21 @@ public static String makeNativeLogFileName(String fileId, 
String writeToken, Str
   }
 
   public static boolean isBaseFile(String path) {
-    if (matchNativeLogFile(path).isPresent()) {
-      return false;
-    }
-    String extension = getFileExtension(path);
-    if (HoodieFileFormat.BASE_FILE_EXTENSIONS.contains(extension)) {
-      return BASE_FILE_PATTERN.matcher(path).matches();
-    }
-    return false;
+    return FileNameParser.parseBaseFile(path)
+        .map(baseFileName -> 
HoodieFileFormat.BASE_FILE_EXTENSIONS.contains(baseFileName.getFileExtension()))

Review Comment:
   **`FSUtils.isBaseFile` is now more permissive than master**, which required 
a full `<fileId>_<writeToken>_<numericCommit>.<ext>` match. The lenient scan 
now accepts e.g. `foo_20240101.parquet` (single underscore, the "hacky" legacy 
branch) and `a_b_xyz.parquet` (non-numeric commit time).
   
   This matters for listing-based classification: `HoodieRepairTool` treats 
`isDataFile` hits absent from commit metadata as dangling and deletes them in 
repair mode, and the MDT bootstrap listing 
(`HoodieTableMetadataUtil.DirectoryInfo`) would index a stray 
`foo_20240101.parquet` into the files partition as a phantom file group. 
Previously the strict pattern ignored such files.
   
   Could we keep the stricter validation for non-external names here, and 
confine the lenient legacy branch to the `HoodieBaseFile` decoding path where 
the historical tolerance is actually needed?



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