slfan1989 commented on code in PR #13913:
URL: https://github.com/apache/iceberg/pull/13913#discussion_r2317568128


##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/procedures/ExpireSnapshotsProcedure.java:
##########
@@ -104,26 +118,28 @@ public ProcedureParameter[] parameters() {
   @Override
   @SuppressWarnings("checkstyle:CyclomaticComplexity")
   public Iterator<Scan> call(InternalRow args) {
-    Identifier tableIdent = toIdentifier(args.getString(0), 
PARAMETERS[0].name());
-    Long olderThanMillis = args.isNullAt(1) ? null : 
DateTimeUtil.microsToMillis(args.getLong(1));
-    Integer retainLastNum = args.isNullAt(2) ? null : args.getInt(2);
-    Integer maxConcurrentDeletes = args.isNullAt(3) ? null : args.getInt(3);
-    Boolean streamResult = args.isNullAt(4) ? null : args.getBoolean(4);
-    long[] snapshotIds = args.isNullAt(5) ? null : 
args.getArray(5).toLongArray();
-    Boolean cleanExpiredMetadata = args.isNullAt(6) ? null : 
args.getBoolean(6);
+    ProcedureInput input = new ProcedureInput(spark(), tableCatalog(), 
PARAMETERS, args);
+    Identifier tableIdent = toIdentifier(input.asString(TABLE_PARAM), 
TABLE_PARAM.name());
+    Long olderThanMillis = input.asTimestampLong(OLDER_THAN_PARAM, null);
+    Integer retainLastNum = input.asInt(RETAIN_LAST_PARAM, null);
+    Integer maxConcurrentDeletes = input.asInt(MAX_CONCURRENT_DELETES_PARAM, 
null);
+    Boolean streamResult = input.asBoolean(STREAM_RESULTS_PARAM, null);

Review Comment:
   Thank you for your suggestion! I find your opinion reasonable. However, 
after reviewing the code before the changes, I noticed that the original author 
used the Boolean type. The goal of our PR is to adjust the way input parameters 
are parsed, so I would prefer to keep the variable type as originally intended 
by the author in this case. Do you think this approach is acceptable?



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