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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
