wangyum commented on code in PR #18002:
URL: https://github.com/apache/iceberg/pull/18002#discussion_r3958026011
##########
core/src/test/java/org/apache/iceberg/util/TestTableScanUtil.java:
##########
@@ -332,6 +315,82 @@ public void testAdaptiveSplitSize() {
.hasMessageStartingWith("Parallelism must be > 0: 0");
}
+ @Test
+ public void testSplitSkipsWrapWhenFileFitsSingleSplit() {
+ // A small splittable file (no offsets) smaller than the target split size
must not be wrapped
+ // in a redundant 1:1 SplitScanTask; split() should return the task itself.
+ BaseFileScanTask task = newScanTask(FileFormat.PARQUET, 64L, null);
+
+ List<FileScanTask> splits = ImmutableList.copyOf(task.split(128L));
+
+ assertThat(splits).hasSize(1);
+ assertThat(splits.get(0)).isSameAs(task);
+ // behavior must be identical to the wrapper it replaces: whole file from
offset 0
+ assertThat(splits.get(0).start()).isEqualTo(0L);
+ assertThat(splits.get(0).length()).isEqualTo(64L);
+ assertThat(splits.get(0).estimatedRowsCount()).isEqualTo(1L);
+ }
+
+ @Test
+ public void testSplitSkipsWrapWhenFileSizeEqualsTargetSplitSize() {
+ // length() == targetSplitSize is still "fits in a single split" and must
not be wrapped
+ BaseFileScanTask task = newScanTask(FileFormat.PARQUET, 128L, null);
+
+ List<FileScanTask> splits = ImmutableList.copyOf(task.split(128L));
+
+ assertThat(splits).hasSize(1);
+ assertThat(splits.get(0)).isSameAs(task);
+ }
+
+ @Test
+ public void testSplitSkipsOffsetsWhenFileFitsSingleSplit() {
+ // Even when a file has valid row-group/stripe offsets, there is no reason
to split it into
+ // per-offset tasks if the whole file already fits within the target split
size.
+ BaseFileScanTask task = newScanTask(FileFormat.PARQUET, 64L,
ImmutableList.of(0L, 32L));
+
+ List<FileScanTask> splits = ImmutableList.copyOf(task.split(128L));
+
+ assertThat(splits).hasSize(1);
+ assertThat(splits.get(0)).isSameAs(task);
+ }
+
+ @Test
+ public void testSplitPreservesOldBehaviorForEmptyFile() {
+ // A zero-length file is not short-circuited; it falls through to the
existing splittable
+ // path. With no split offsets, FixedSizeSplitScanTaskIterator yields zero
splits, matching
+ // the behavior before this change (hasNext() == remainingLength > 0).
+ BaseFileScanTask task = newScanTask(FileFormat.PARQUET, 0L, null);
+
+ List<FileScanTask> splits = ImmutableList.copyOf(task.split(128L));
+
+ assertThat(splits).isEmpty();
+ }
+
+ private BaseFileScanTask newScanTask(
+ FileFormat format, long sizeInBytes, List<Long> splitOffsets) {
+ DataFiles.Builder builder =
+ DataFiles.builder(TestBase.SPEC)
+ .withPath("/path/to/" + format.addExtension("data-a"))
+ .withFormat(format)
+ .withFileSizeInBytes(sizeInBytes)
+ .withPartitionPath("data_bucket=0")
+ .withRecordCount(1);
+
+ if (splitOffsets != null) {
+ builder.withSplitOffsets(splitOffsets);
+ }
+
+ ResidualEvaluator residualEvaluator =
+ ResidualEvaluator.of(TestBase.SPEC, Expressions.alwaysTrue(), false);
+
+ return new BaseFileScanTask(
+ builder.build(),
+ null,
+ SchemaParser.toJson(TestBase.SCHEMA),
+ PartitionSpecParser.toJson(TestBase.SPEC),
+ residualEvaluator);
+ }
+
Review Comment:
Added testSplitFallsThroughWhenFileExceedsTargetSplitSize to guard the
fall-through path
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
