rdblue commented on code in PR #18699:
URL: https://github.com/apache/iceberg/pull/18699#discussion_r4419133532


##########
core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java:
##########
@@ -362,22 +333,12 @@ private ManifestFile copyManifest(ManifestFile manifest) {
    */
   protected void validateAddedDataFiles(
       TableMetadata base, Long startingSnapshotId, PartitionSet partitionSet, 
Snapshot parent) {
-    CloseableIterable<ManifestEntry<DataFile>> conflictEntries =
-        addedDataFiles(base, startingSnapshotId, null, partitionSet, parent);
-
-    try (CloseableIterator<ManifestEntry<DataFile>> conflicts = 
conflictEntries.iterator()) {
-      if (conflicts.hasNext()) {
-        throw new ValidationException(
-            "Found conflicting files that can contain records matching 
partitions %s: %s",
-            partitionSet,
-            Iterators.toString(
-                Iterators.transform(conflicts, entry -> 
entry.file().location().toString())));
-      }
-
-    } catch (IOException e) {
-      throw new UncheckedIOException(
-          String.format("Failed to validate no appends matching %s", 
partitionSet), e);
-    }
+    SnapshotValidator.newValidator()
+        .io(ops().io())
+        .startingSnapshotId(startingSnapshotId)
+        .caseSensitive(caseSensitive)
+        .forWindow(base, parent)
+        .validateAddedDataFiles(partitionSet);

Review Comment:
   Is `partitionSet` an alternative to `conflictDetectionFilter`? Should it be 
configured the same way so that there is just one `validateAddedDataFiles()` 
call instead of two? Having two means that the conflict detection filter is 
ignored by the validator when the partition set version is called. I see that 
it is not set in actual uses, but the API allows you to set 
`conflictDetectionFilter` and also call `validateAddedDataFiles(PartitionSet)`.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to