coderfender commented on code in PR #12824: URL: https://github.com/apache/iceberg/pull/12824#discussion_r2054973916
########## spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/actions/RewriteDataFilesSparkAction.java: ########## @@ -407,15 +409,49 @@ private Builder doExecuteWithPartialProgress( Stream<RewriteFileGroup> toGroupStream( RewriteExecutionContext ctx, Map<StructLike, List<List<FileScanTask>>> groupsByPartition) { - return groupsByPartition.entrySet().stream() + + if (maxFilesToRewrite == 0) { Review Comment: Sure ! In terms of using a default / magic value , 1. I just set the max_files_to_rewrite as an Integer as supposed to an int (to avoid magic 0's) and handle default use case as null / Or I could init to a -1) 2. Remove brnaching but make sure there are no side effects (as not to cap the number of files should the user choose not provide an option) 3. Clean up the code and not use streams at all ? (I dont believe this is would be a good ROI given @pvary 's overhaul of the Rewrite action) -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org