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) 
   2.  Add no additional side effects (i.e cap the number of files should the 
user choose not provide an option at all to not have side effects)
   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 though) 



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