zhoujinsong commented on code in PR #3175:
URL: https://github.com/apache/amoro/pull/3175#discussion_r1804604702


##########
amoro-format-iceberg/src/main/java/org/apache/amoro/io/reader/CombinedDeleteFilter.java:
##########
@@ -115,34 +126,19 @@ protected CombinedDeleteFilter(
     this.dataRecordCnt =
         
Arrays.stream(rewriteFilesInput.dataFiles()).mapToLong(ContentFile::recordCount).sum();
     ImmutableList.Builder<DeleteFile> posDeleteBuilder = 
ImmutableList.builder();
-    ImmutableList.Builder<DeleteFile> eqDeleteBuilder = 
ImmutableList.builder();
     if (rewriteFilesInput.deleteFiles() != null) {
-      String firstDeleteFilePath = null;
       for (ContentFile<?> delete : rewriteFilesInput.deleteFiles()) {
         switch (delete.content()) {
           case POSITION_DELETES:
             posDeleteBuilder.add(ContentFiles.asDeleteFile(delete));
             break;
           case EQUALITY_DELETES:
-            if (deleteIds.isEmpty()) {
-              deleteIds = 
ImmutableSet.copyOf(ContentFiles.asDeleteFile(delete).equalityFieldIds());
-              firstDeleteFilePath = delete.path().toString();
-            } else {
-              Set<Integer> currentDeleteIds =
-                  
ImmutableSet.copyOf(ContentFiles.asDeleteFile(delete).equalityFieldIds());
-              if (!deleteIds.equals(currentDeleteIds)) {
-                throw new IllegalArgumentException(
-                    String.format(
-                        "Equality delete files have different delete fields, 
first equality field ids:[%s],"
-                            + " current equality field ids:[%s], first delete 
file path:[%s], "
-                            + " current delete file path: [%s].",
-                        deleteIds,
-                        currentDeleteIds,
-                        firstDeleteFilePath,
-                        delete.path().toString()));
-              }
-            }
-            eqDeleteBuilder.add(ContentFiles.asDeleteFile(delete));
+            DeleteFile deleteFile = ContentFiles.asDeleteFile(delete);
+            Set<Integer> eqFieldIds = 
Sets.newHashSet(delete.equalityFieldIds());
+            deleteIds.addAll(eqFieldIds);
+            eqDeleteFilesByDeleteIds.put(eqFieldIds, deleteFile);
+            deleteSchemaByDeleteIds.putIfAbsent(

Review Comment:
   How about using `computeIfAbsent` here to avoid building schema for every 
file as its cost a bit high.



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