[GitHub] [iceberg] kbendick commented on a change in pull request #1285: Core: fix serialization issue in BaseCombinedScanTask with Kyro (No refactor)

[GitBox <gi...@apache.org>]

kbendick commented on a change in pull request #1285:
URL: https://github.com/apache/iceberg/pull/1285#discussion_r464851326



##########
File path: core/src/main/java/org/apache/iceberg/BaseCombinedScanTask.java
##########
@@ -26,19 +26,19 @@
 import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
 
 public class BaseCombinedScanTask implements CombinedScanTask {
-  private final List<FileScanTask> tasks;
+  private final FileScanTask[] tasks;
 
   public BaseCombinedScanTask(FileScanTask... tasks) {
-    this.tasks = ImmutableList.copyOf(tasks);
+    this.tasks = tasks;
   }
 
   public BaseCombinedScanTask(List<FileScanTask> tasks) {
-    this.tasks = ImmutableList.copyOf(tasks);
+    this.tasks = tasks.stream().toArray(FileScanTask[]::new);

Review comment:
       In Guava, `ImmutableList.copyOf()` will throw on null input. That same behavior isn't mentioned here. Should we document it, either via a `Preconditions` check or via an annotation such as `@ParametersAreNonNullByDefault` which Guava uses.
   
   Note: I can't speak for all versions of Guava obviously (I doubt anybody could at this point), but I found this isue mentioning it https://github.com/google/guava/issues/2470




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org