yyanyy commented on a change in pull request #2294:
URL: https://github.com/apache/iceberg/pull/2294#discussion_r601861488
##########
File path: core/src/main/java/org/apache/iceberg/BaseRewriteFiles.java
##########
@@ -40,19 +40,55 @@ protected String operation() {
return DataOperations.REPLACE;
}
+ private void verifyInputAndOutputFiles(Set<DataFile> dataFilesToDelete,
Set<DeleteFile> deleteFilesToDelete,
+ Set<DataFile> dataFilesToAdd,
Set<DeleteFile> deleteFilesToAdd) {
+ int filesToDelete = 0;
+ if (dataFilesToDelete != null) {
+ filesToDelete += dataFilesToDelete.size();
+ }
+
+ if (deleteFilesToDelete != null) {
+ filesToDelete += deleteFilesToDelete.size();
+ }
+
+ Preconditions.checkArgument(filesToDelete > 0, "Files to delete cannot be
null or empty");
+
+ if (deleteFilesToDelete == null || deleteFilesToDelete.isEmpty()) {
+ // When there is no delete files in the rewrite action, data files to
add cannot be null or empty.
+ Preconditions.checkArgument(dataFilesToAdd != null &&
dataFilesToAdd.size() > 0,
+ "Data files to add can not be null or empty because there's no
delete file to rewrite");
+ Preconditions.checkArgument(deleteFilesToAdd == null ||
deleteFilesToAdd.isEmpty(),
+ "Delete files to add must be null or empty because there's no delete
file to rewrite");
+ }
+ }
+
@Override
- public RewriteFiles rewriteFiles(Set<DataFile> filesToDelete, Set<DataFile>
filesToAdd) {
- Preconditions.checkArgument(filesToDelete != null &&
!filesToDelete.isEmpty(),
- "Files to delete cannot be null or empty");
- Preconditions.checkArgument(filesToAdd != null && !filesToAdd.isEmpty(),
Review comment:
I agree that they can be empty, I guess my major point is that since it
is the new API we introduced here that allows empty input, we can enforce
inputs to be not null by adding a precondition check at the beginning of the
method to fail if null is passed in, so that we don't have to do all the `!=
null` everywhere to make the code a bit more clean, while still allowing
`ImmutableSet.of()` which should logically be equivalent to null.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
