u70b3 commented on code in PR #16449:
URL: https://github.com/apache/iceberg/pull/16449#discussion_r3683217726
##########
core/src/main/java/org/apache/iceberg/rest/responses/BaseScanTaskResponse.java:
##########
@@ -93,20 +89,7 @@ public B withFileScanTasks(List<FileScanTask> tasks) {
return self();
Review Comment:
Could `withFileScanTasks(null)` clear the builder's derived `deleteFiles` as
well? I may be misunderstanding the builder contract, but I reproduced this by
creating a completed response whose task has a delete file, then calling
`response.toBuilder().withPlanStatus(FAILED).withFileScanTasks(null).build()`.
It throws `IllegalArgumentException: deleteFiles should only be returned with
fileScanTasks that reference them` because the copied delete files remain.
Setting `deleteFiles` to `null` when tasks are cleared made the focused test
pass. I ran: `./gradlew :iceberg-core:test --tests
"org.apache.iceberg.rest.responses.TestPlanTableScanResponseParser.toBuilderClearsDeleteFilesWhenClearingFileScanTasks"`.
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/SparkTableUtil.java:
##########
@@ -595,6 +379,8 @@ public static void importSparkTable(
"Table %s does not exist", sourceTableIdentWithDB);
}
+ ensureNameMappingPresent(targetTable);
Review Comment:
Could we also ensure the name mapping in the deepest retained
`importSparkPartitions` overload (or otherwise ensure that every public
overload reaches this initialization)? I may be missing an intended
precondition, but I reproduced this on the current PR head by adapting the
existing `TestIdentityPartitionData` setup to call public `getPartitions` +
`importSparkPartitions` directly. Both Parquet parameterizations failed: the
partition columns remained correct, but every `id` and `message` value was read
as `null`. Adding `ensureNameMappingPresent(targetTable)` to the deepest
overload made all 5 `testFullProjection` parameterizations pass. I ran:
`./gradlew :iceberg-spark:iceberg-spark-4.1_2.13:test --tests
"org.apache.iceberg.spark.source.TestIdentityPartitionData.testFullProjection"`.
--
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]
