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

Reply via email to