gaborkaszab commented on code in PR #13569:
URL: https://github.com/apache/iceberg/pull/13569#discussion_r2215032265


##########
flink/v2.0/flink/src/main/java/org/apache/iceberg/flink/maintenance/api/ExpireSnapshots.java:
##########
@@ -46,6 +46,7 @@ public static class Builder extends 
MaintenanceTaskBuilder<ExpireSnapshots.Build
     private Integer numSnapshots = null;
     private Integer planningWorkerPoolSize;
     private int deleteBatchSize = DELETE_BATCH_SIZE_DEFAULT;
+    private Boolean cleanExpiredMetadata = null;

Review Comment:
   That's a good observation, @pvary !
   In fact, in case we want to change the default value then we have to change 
it 3 places: Flink, Spark and Java core, otherwise engines using the Java API 
only (Trino, Impala, Hive) would see a different default value than in Spark or 
Flink.
   Following this thinking, I think we should also re-consider the type of the 
flag in the Spark proc/action: null would mean that the value is not provided 
by the user and we go with the default value in Java core, otherwise we use the 
value explicitly provided by the user. This way changing the default value 
would only be needed in one place, in Java core.
   
   WDYT @nastra @mxm ?



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