hudi-agent commented on code in PR #19497:
URL: https://github.com/apache/hudi/pull/19497#discussion_r3870961484


##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieMetadataTableValidator.java:
##########
@@ -810,21 +811,69 @@ List<String> validatePartitions(HoodieSparkEngineContext 
engineContext, StorageP
         }
       }
       if (misMatch.get()) {
+        // The partition lists genuinely disagree, so this run fails either 
way. Before reporting it,
+        // validate the partitions both sides do agree on: throwing here skips 
the per-partition file
+        // validation the caller would otherwise run, which leaves the 
operator knowing only that the
+        // partition lists differ and nothing about whether the rest of the 
table is consistent.
+        List<String> commonPartitions = new 
ArrayList<>(allPartitionPathsFromFS);
+        commonPartitions.retainAll(allPartitionPathsMeta);
         String message = "Compare Partitions Failed! " + " Additional "
             + additionalFromFS.size() + " partitions from FS, but missing from 
MDT : \""
             + toStringWithThreshold(additionalFromFS, cfg.logDetailMaxLength)
             + "\" and additional " + actualAdditionalPartitionsInMDT.size()
             + " partitions from MDT, but missing from FS listing : \""
             + toStringWithThreshold(actualAdditionalPartitionsInMDT, 
cfg.logDetailMaxLength)
             + "\".\n All " + allPartitionPathsFromFS.size() + " partitions 
from FS listing "
-            + toStringWithThreshold(allPartitionPathsFromFS, 
cfg.logDetailMaxLength);
+            + toStringWithThreshold(allPartitionPathsFromFS, 
cfg.logDetailMaxLength)
+            + "\n" + validateFilesInCommonPartitions(engineContext, 
metaClient, commonPartitions, baseFilesForCleaning);
         log.error(message);
         throw new HoodieValidationException(message);
       }
     }
     return allPartitionPathsMeta;
   }
 
+  /**
+   * Validates the file listing for the partitions present on both sides, and 
summarises the outcome.
+   *
+   * <p>Only called once a partition mismatch has already been detected and 
this run is going to fail.
+   * The result is reported alongside the mismatch rather than thrown, so that 
the partition list
+   * difference stays the reported cause; a file level problem found here is 
extra detail about a run
+   * that was failing anyway. Any error raised while validating a common 
partition is captured for the

Review Comment:
   🤖 nit: could you rename this to `summarizeFileValidationForCommonPartitions` 
(or similar)? Every other `validate*` method in this class signals failure by 
throwing, so a future reader might expect this one to throw too — returning a 
`String` summary is unexpected from the name alone.
   
   <sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag 
quality.</i></sub>



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