dramaticlly commented on code in PR #16449:
URL: https://github.com/apache/iceberg/pull/16449#discussion_r3693509382
##########
core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponse.java:
##########
@@ -65,6 +65,29 @@ public static Builder builder() {
return new Builder();
}
+ /**
+ * Returns a new builder pre-populated with the given partition specs map.
Required for server
+ * responses that serialize {@code fileScanTasks} or {@code deleteFiles};
the specs are used only
+ * to serialize partition data and are never written to the response payload.
+ */
+ public static Builder builder(Map<Integer, PartitionSpec> specsById) {
+ return new Builder().withSpecsById(specsById);
+ }
+
+ /**
+ * Returns a builder pre-populated with this response's fields, suitable for
producing a copy with
+ * one or more fields modified.
+ */
+ public Builder toBuilder() {
+ return new Builder()
+ .withPlanStatus(planStatus)
+ .withErrorResponse(errorResponse)
+ .withPlanTasks(planTasks())
+ .withFileScanTasks(fileScanTasks())
+ .withCredentials(credentials())
+ .withSpecsById(specsById());
+ }
Review Comment:
I added this to provide a way for class outside the package to create the
builder correctly as now the specById is reduced to protected scope. Current
user are `org.apache.iceberg.rest.RESTServerCatalogAdapter` and
`org.apache.iceberg.rest.TestRESTScanPlanning`
##########
core/src/main/java/org/apache/iceberg/rest/responses/BaseScanTaskResponse.java:
##########
@@ -85,28 +81,15 @@ public B withPlanTasks(List<String> tasks) {
public B withFileScanTasks(List<FileScanTask> tasks) {
this.fileScanTasks = tasks;
- if (fileScanTasks != null) {
- this.deleteFiles =
- DeleteFileSet.of(
- () -> tasks.stream().flatMap(task ->
task.deletes().stream()).iterator());
- }
+ this.deleteFiles =
+ tasks == null
+ ? null
+ : DeleteFileSet.of(
+ () -> tasks.stream().flatMap(task ->
task.deletes().stream()).iterator());
Review Comment:
the logic is revised slightly so that now we can clean `this.deleteFiles`
when passing null tasks. This is needed as we are removing the seter of
`withDeleteFiles` in the same PR, so need a way to reset the internal
deleteFiles if they are not longer applicable. Before the change it will only
set `this.deleteFiles` if we have a valid task but not be able to clear
--
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]
