anoopj commented on code in PR #18699:
URL: https://github.com/apache/iceberg/pull/18699#discussion_r4464303168
##########
core/src/main/java/org/apache/iceberg/MergingSnapshotProducer.java:
##########
@@ -708,161 +481,33 @@ protected void validateDeletedDataFiles(
*/
protected void validateDeletedDataFiles(
TableMetadata base, Long startingSnapshotId, PartitionSet partitionSet,
Snapshot parent) {
- CloseableIterable<ManifestEntry<DataFile>> conflictEntries =
- deletedDataFiles(base, startingSnapshotId, null, partitionSet, parent);
-
- try (CloseableIterator<ManifestEntry<DataFile>> conflicts =
conflictEntries.iterator()) {
- if (conflicts.hasNext()) {
- throw new ValidationException(
- "Found conflicting deleted files that can apply to records
matching %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);
- }
- }
-
- /**
- * Returns an iterable of files matching a filter have been added to the
table since a starting
- * snapshot.
- *
- * @param base table metadata to validate
- * @param startingSnapshotId id of the snapshot current at the start of the
operation
- * @param dataFilter an expression used to find deleted data files
- * @param partitionSet a set of partitions to find deleted data files
- * @param parent ending snapshot on the branch being validated
- */
- private CloseableIterable<ManifestEntry<DataFile>> deletedDataFiles(
- TableMetadata base,
- Long startingSnapshotId,
- Expression dataFilter,
- PartitionSet partitionSet,
- Snapshot parent) {
- // if there is no current table state, no files have been deleted
- if (parent == null) {
- return CloseableIterable.empty();
- }
-
- Pair<List<ManifestFile>, Set<Long>> history =
- validationHistory(
- base,
- startingSnapshotId,
- VALIDATE_DATA_FILES_EXIST_OPERATIONS,
- ManifestContent.DATA,
- parent);
- List<ManifestFile> manifests = history.first();
- Set<Long> newSnapshots = history.second();
-
- ManifestGroup manifestGroup =
- new ManifestGroup(ops().io(), manifests, ImmutableList.of())
- .caseSensitive(caseSensitive)
- .filterManifestEntries(entry ->
newSnapshots.contains(entry.snapshotId()))
- .filterManifestEntries(entry ->
entry.status().equals(ManifestEntry.Status.DELETED))
- .specsById(base.specsById())
- .ignoreExisting();
-
- if (dataFilter != null) {
- manifestGroup = manifestGroup.filterData(dataFilter);
- }
-
- if (partitionSet != null) {
- manifestGroup =
- manifestGroup.filterManifestEntries(
- entry -> partitionSet.contains(entry.file().specId(),
entry.file().partition()));
- }
-
- return manifestGroup.entries();
+ CommitValidator.newValidator()
+ .io(ops().io())
+ .startingSnapshotId(startingSnapshotId)
+ .caseSensitive(caseSensitive)
+ .partitionSet(partitionSet)
+ .setParent(base, parent)
+ .validateNoConflictingDeletedDataFiles();
}
protected void setNewDataFilesDataSequenceNumber(long sequenceNumber) {
Review Comment:
Moved it out of the validation methods: to before the validations
--
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]
