You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by am...@apache.org on 2026/08/26 15:23:14 UTC
(iceberg) branch main updated: Core: Reduce visibility on scan planning response builder for specsById and deleteFiles (#17638)
This is an automated email from the ASF dual-hosted git repository.
amogh-jahagirdar pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new 3bf74aae1d Core: Reduce visibility on scan planning response builder for specsById and deleteFiles (#17638)
3bf74aae1d is described below
commit 3bf74aae1d7cf00d5f7964fd99629914ab05b27c
Author: Hongyue/Steve Zhang <ho...@gmail.com>
AuthorDate: Wed Aug 26 08:23:08 2026 -0700
Core: Reduce visibility on scan planning response builder for specsById and deleteFiles (#17638)
---
.palantir/revapi.yml | 124 +++++++++++++++++++++
.../org/apache/iceberg/rest/CatalogHandlers.java | 14 +--
.../rest/requests/PlanTableScanRequest.java | 7 +-
.../rest/responses/BaseScanTaskResponse.java | 43 ++-----
.../responses/FetchPlanningResultResponse.java | 23 ++++
.../rest/responses/FetchScanTasksResponse.java | 9 ++
.../rest/responses/PlanTableScanResponse.java | 31 +++++-
.../apache/iceberg/rest/TestRESTScanPlanning.java | 46 +++-----
.../TestFetchPlanningResultResponseParser.java | 12 --
.../TestFetchScanTasksResponseParser.java | 36 ++++--
.../responses/TestPlanTableScanResponseParser.java | 41 +++++--
.../iceberg/rest/RESTServerCatalogAdapter.java | 12 +-
12 files changed, 269 insertions(+), 129 deletions(-)
diff --git a/.palantir/revapi.yml b/.palantir/revapi.yml
index 3fe745d303..b7045c8639 100644
--- a/.palantir/revapi.yml
+++ b/.palantir/revapi.yml
@@ -535,6 +535,64 @@ acceptedBreaks:
old: "class org.apache.iceberg.data.avro.DataReader<T extends java.lang.Object>"
justification: "Removing deprecated DataReader and RawDecoder constructor scheduled\
\ for removal in 1.12.0"
+ - code: "java.element.noLongerDeprecated"
+ old: "method B org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::withSpecsById(java.util.Map<java.lang.Integer,\
+ \ org.apache.iceberg.PartitionSpec>)"
+ new: "method B org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::withSpecsById(java.util.Map<java.lang.Integer,\
+ \ org.apache.iceberg.PartitionSpec>)"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.element.noLongerDeprecated"
+ old: "method java.util.List<org.apache.iceberg.DeleteFile> org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::deleteFiles()"
+ new: "method java.util.List<org.apache.iceberg.DeleteFile> org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::deleteFiles()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.element.noLongerDeprecated"
+ old: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B, R extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::specsById()"
+ new: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B, R extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::specsById()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.element.noLongerDeprecated"
+ old: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse::specsById()"
+ new: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse::specsById()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.element.noLongerDeprecated"
+ old: "method void org.apache.iceberg.rest.requests.PlanTableScanRequest.Builder::<init>()"
+ new: "method void org.apache.iceberg.rest.requests.PlanTableScanRequest.Builder::<init>()"
+ justification: "Reducing visibility of scan-planning request internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.element.noLongerDeprecated"
+ old: "method void org.apache.iceberg.rest.responses.PlanTableScanResponse.Builder::<init>()"
+ new: "method void org.apache.iceberg.rest.responses.PlanTableScanResponse.Builder::<init>()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
- code: "java.field.removed"
old: "field org.apache.iceberg.PartitionStatsHandler.DATA_FILE_COUNT"
justification: "Removed deprecated functionality for partition stats"
@@ -649,6 +707,14 @@ acceptedBreaks:
\ java.lang.String)"
justification: "A manifest can only be rewritten together with its measured\
\ length, which the set of manifest paths cannot carry"
+ - code: "java.method.removed"
+ old: "method B org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::withDeleteFiles(java.util.List<org.apache.iceberg.DeleteFile>)"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
- code: "java.method.removed"
old: "method boolean org.apache.iceberg.util.TableScanUtil::hasDeletes(org.apache.iceberg.CombinedScanTask)"
justification: "Removed deprecated API scheduled for removal in 1.12.0"
@@ -786,6 +852,64 @@ acceptedBreaks:
- code: "java.method.removed"
old: "method void org.apache.iceberg.rest.ResourcePaths::<init>(java.lang.String)"
justification: "Removed deprecated functionality scheduled for removal in 1.12.0"
+ - code: "java.method.visibilityReduced"
+ old: "method B org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::withSpecsById(java.util.Map<java.lang.Integer,\
+ \ org.apache.iceberg.PartitionSpec>)"
+ new: "method B org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::withSpecsById(java.util.Map<java.lang.Integer,\
+ \ org.apache.iceberg.PartitionSpec>)"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.method.visibilityReduced"
+ old: "method java.util.List<org.apache.iceberg.DeleteFile> org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::deleteFiles()"
+ new: "method java.util.List<org.apache.iceberg.DeleteFile> org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B,\
+ \ R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R\
+ \ extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::deleteFiles()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.method.visibilityReduced"
+ old: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B, R extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::specsById()"
+ new: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse.Builder<B, R extends\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse>, R>, R extends org.apache.iceberg.rest.responses.BaseScanTaskResponse>::specsById()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.method.visibilityReduced"
+ old: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse::specsById()"
+ new: "method java.util.Map<java.lang.Integer, org.apache.iceberg.PartitionSpec>\
+ \ org.apache.iceberg.rest.responses.BaseScanTaskResponse::specsById()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.method.visibilityReduced"
+ old: "method void org.apache.iceberg.rest.requests.PlanTableScanRequest.Builder::<init>()"
+ new: "method void org.apache.iceberg.rest.requests.PlanTableScanRequest.Builder::<init>()"
+ justification: "Reducing visibility of scan-planning request internals deprecated\
+ \ in 1.11.0 for 1.12.0"
+ - code: "java.method.visibilityReduced"
+ old: "method void org.apache.iceberg.rest.responses.PlanTableScanResponse.Builder::<init>()"
+ new: "method void org.apache.iceberg.rest.responses.PlanTableScanResponse.Builder::<init>()"
+ justification: "Reducing visibility of scan-planning response internals deprecated\
+ \ in 1.11.0 for 1.12.0"
org.apache.iceberg:iceberg-data:
- code: "java.class.removed"
old: "class org.apache.iceberg.data.BaseFileWriterFactory<T extends java.lang.Object>"
diff --git a/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java b/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
index 61ac082a30..13089fc07d 100644
--- a/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
+++ b/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
@@ -848,10 +848,9 @@ public class CatalogHandlers {
table.uuid().toString(),
tasksPerPlanTask.applyAsInt(configuredScan),
request.minRowsRequested());
- return PlanTableScanResponse.builder()
+ return PlanTableScanResponse.builder(table.specs())
.withPlanId(asyncPlanId)
.withPlanStatus(PlanStatus.SUBMITTED)
- .withSpecsById(table.specs())
.build();
}
@@ -868,11 +867,10 @@ public class CatalogHandlers {
? Collections.emptyList()
: IN_MEMORY_PLANNING_STATE.nextPlanTask(initial.second());
PlanTableScanResponse.Builder builder =
- PlanTableScanResponse.builder()
+ PlanTableScanResponse.builder(table.specs())
.withPlanStatus(PlanStatus.COMPLETED)
.withPlanId(planId)
- .withFileScanTasks(initial.first())
- .withSpecsById(table.specs());
+ .withFileScanTasks(initial.first());
if (!nextPlanTasks.isEmpty()) {
builder.withPlanTasks(nextPlanTasks);
@@ -898,11 +896,10 @@ public class CatalogHandlers {
}
Pair<List<FileScanTask>, String> initial = IN_MEMORY_PLANNING_STATE.initialScanTasksFor(planId);
- return FetchPlanningResultResponse.builder()
+ return FetchPlanningResultResponse.builder(table.specs())
.withPlanStatus(PlanStatus.COMPLETED)
.withFileScanTasks(initial.first())
.withPlanTasks(IN_MEMORY_PLANNING_STATE.nextPlanTask(initial.second()))
- .withSpecsById(table.specs())
.build();
}
@@ -920,10 +917,9 @@ public class CatalogHandlers {
String planTask = request.planTask();
List<FileScanTask> fileScanTasks = IN_MEMORY_PLANNING_STATE.fileScanTasksForPlanTask(planTask);
- return FetchScanTasksResponse.builder()
+ return FetchScanTasksResponse.builder(table.specs())
.withFileScanTasks(fileScanTasks)
.withPlanTasks(IN_MEMORY_PLANNING_STATE.nextPlanTask(planTask))
- .withSpecsById(table.specs())
.build();
}
diff --git a/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java b/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java
index 6729f25d7f..7e3aff7ea1 100644
--- a/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java
+++ b/core/src/main/java/org/apache/iceberg/rest/requests/PlanTableScanRequest.java
@@ -143,12 +143,7 @@ public class PlanTableScanRequest implements RESTRequest {
private List<String> statsFields;
private Long minRowsRequested;
- /**
- * @deprecated since 1.11.0, visibility will be reduced in 1.12.0; use {@link
- * PlanTableScanRequest#builder()} instead.
- */
- @Deprecated
- public Builder() {}
+ private Builder() {}
public Builder withSnapshotId(Long withSnapshotId) {
this.snapshotId = withSnapshotId;
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/BaseScanTaskResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/BaseScanTaskResponse.java
index e83cacc484..cdf682b35f 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/BaseScanTaskResponse.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/BaseScanTaskResponse.java
@@ -57,11 +57,7 @@ public abstract class BaseScanTaskResponse implements RESTResponse {
return deleteFiles == null ? null : Lists.newArrayList(deleteFiles.iterator());
}
- /**
- * @deprecated since 1.11.0, visibility will be reduced in 1.12.0.
- */
- @Deprecated
- public Map<Integer, PartitionSpec> specsById() {
+ protected Map<Integer, PartitionSpec> specsById() {
return specsById;
}
@@ -85,28 +81,15 @@ public abstract class BaseScanTaskResponse implements RESTResponse {
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());
return self();
}
- /**
- * @deprecated since 1.11.0, will be removed in 1.12.0.
- */
- @Deprecated
- public B withDeleteFiles(List<DeleteFile> deleteFilesList) {
- this.deleteFiles = DeleteFileSet.of(deleteFilesList);
- return self();
- }
-
- /**
- * @deprecated since 1.11.0, visibility will be reduced in 1.12.0.
- */
- @Deprecated
- public B withSpecsById(Map<Integer, PartitionSpec> specs) {
+ protected B withSpecsById(Map<Integer, PartitionSpec> specs) {
this.specsById = specs;
return self();
}
@@ -119,19 +102,11 @@ public abstract class BaseScanTaskResponse implements RESTResponse {
return fileScanTasks;
}
- /**
- * @deprecated since 1.11.0, visibility will be reduced in 1.12.0.
- */
- @Deprecated
- public List<DeleteFile> deleteFiles() {
+ protected List<DeleteFile> deleteFiles() {
return deleteFiles == null ? null : Lists.newArrayList(deleteFiles.iterator());
}
- /**
- * @deprecated since 1.11.0, visibility will be reduced in 1.12.0.
- */
- @Deprecated
- public Map<Integer, PartitionSpec> specsById() {
+ protected Map<Integer, PartitionSpec> specsById() {
return specsById;
}
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponse.java
index 2e176aac65..2d219c36d0 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponse.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponse.java
@@ -65,6 +65,29 @@ public class FetchPlanningResultResponse extends BaseScanTaskResponse {
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());
+ }
+
@Override
public void validate() {
Preconditions.checkArgument(planStatus() != null, "Invalid status: null");
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponse.java
index 6dcd85e6d3..fc78c973f0 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponse.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponse.java
@@ -53,6 +53,15 @@ public class FetchScanTasksResponse extends BaseScanTaskResponse {
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);
+ }
+
public static class Builder
extends BaseScanTaskResponse.Builder<Builder, FetchScanTasksResponse> {
private Builder() {}
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponse.java
index d0ac222c30..1b9ff9d10a 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponse.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponse.java
@@ -116,18 +116,37 @@ public class PlanTableScanResponse extends BaseScanTaskResponse {
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)
+ .withPlanId(planId)
+ .withErrorResponse(errorResponse)
+ .withPlanTasks(planTasks())
+ .withFileScanTasks(fileScanTasks())
+ .withCredentials(credentials())
+ .withSpecsById(specsById());
+ }
+
public static class Builder extends BaseScanTaskResponse.Builder<Builder, PlanTableScanResponse> {
private PlanStatus planStatus;
private String planId;
private ErrorResponse errorResponse;
private final List<Credential> credentials = Lists.newArrayList();
- /**
- * @deprecated since 1.11.0, visibility will be reduced in 1.12.0; use {@link
- * PlanTableScanResponse#builder()} instead.
- */
- @Deprecated
- public Builder() {}
+ private Builder() {}
public Builder withPlanStatus(PlanStatus status) {
this.planStatus = status;
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTScanPlanning.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTScanPlanning.java
index 117bf7fac2..172fcae769 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTScanPlanning.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTScanPlanning.java
@@ -62,7 +62,6 @@ import org.apache.iceberg.io.CloseableIterator;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
-import org.apache.iceberg.rest.credentials.Credential;
import org.apache.iceberg.rest.credentials.ImmutableCredential;
import org.apache.iceberg.rest.requests.PlanTableScanRequest;
import org.apache.iceberg.rest.responses.ConfigResponse;
@@ -1371,10 +1370,12 @@ public class TestRESTScanPlanning extends TestBaseWithRESTServer {
&& planResp.planStatus() == PlanStatus.COMPLETED) {
return castResponse(
responseType,
- PlanTableScanResponse.builder()
+ planResp.toBuilder()
.withPlanStatus(PlanStatus.FAILED)
.withErrorResponse(serverError)
- .withSpecsById(planResp.specsById())
+ .withPlanId(null)
+ .withFileScanTasks(null)
+ .withPlanTasks(null)
.build());
}
if (response instanceof FetchPlanningResultResponse) {
@@ -1474,39 +1475,24 @@ public class TestRESTScanPlanning extends TestBaseWithRESTServer {
if (response instanceof PlanTableScanResponse resp
&& PlanStatus.COMPLETED == resp.planStatus()) {
return (T)
- PlanTableScanResponse.builder()
- .withPlanStatus(resp.planStatus())
- .withPlanId(resp.planId())
- .withPlanTasks(resp.planTasks())
- .withFileScanTasks(resp.fileScanTasks())
+ resp.toBuilder()
.withCredentials(
- ImmutableList.<Credential>builder()
- .addAll(resp.credentials())
- .add(
- ImmutableCredential.builder()
- .prefix("dummy")
- .putConfig("dummyKey", "dummyVal")
- .build())
- .build())
- .withSpecsById(resp.specsById())
+ ImmutableList.of(
+ ImmutableCredential.builder()
+ .prefix("dummy")
+ .putConfig("dummyKey", "dummyVal")
+ .build()))
.build();
} else if (response instanceof FetchPlanningResultResponse resp
&& PlanStatus.COMPLETED == resp.planStatus()) {
return (T)
- FetchPlanningResultResponse.builder()
- .withPlanStatus(resp.planStatus())
- .withFileScanTasks(resp.fileScanTasks())
- .withPlanTasks(resp.planTasks())
- .withSpecsById(resp.specsById())
+ resp.toBuilder()
.withCredentials(
- ImmutableList.<Credential>builder()
- .addAll(resp.credentials())
- .add(
- ImmutableCredential.builder()
- .prefix("dummy")
- .putConfig("dummyKey", "dummyVal")
- .build())
- .build())
+ ImmutableList.of(
+ ImmutableCredential.builder()
+ .prefix("dummy")
+ .putConfig("dummyKey", "dummyVal")
+ .build()))
.build();
}
diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchPlanningResultResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchPlanningResultResponseParser.java
index 841083f88b..f0eb1cfdf2 100644
--- a/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchPlanningResultResponseParser.java
+++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchPlanningResultResponseParser.java
@@ -144,18 +144,6 @@ public class TestFetchPlanningResultResponseParser {
@Test
public void roundTripSerdeWithInvalidPlanStatusSubmittedWithDeleteFilesNoFileScanTasksPresent() {
- PlanStatus planStatus = PlanStatus.fromName("submitted");
- assertThatThrownBy(
- () -> {
- FetchPlanningResultResponse.builder()
- .withPlanStatus(planStatus)
- .withDeleteFiles(List.of(FILE_A_DELETES))
- .build();
- })
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage(
- "Invalid response: deleteFiles should only be returned with fileScanTasks that reference them");
-
String invalidJson =
"{\"status\":\"submitted\","
+ "\"delete-files\":[{\"spec-id\":0,\"content\":\"position-deletes\","
diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchScanTasksResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchScanTasksResponseParser.java
index 7b44b3533a..13910e574e 100644
--- a/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchScanTasksResponseParser.java
+++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestFetchScanTasksResponseParser.java
@@ -83,16 +83,6 @@ public class TestFetchScanTasksResponseParser {
@Test
public void roundTripSerdeWithDeleteFilesNoFileScanTasksPresent() {
- assertThatThrownBy(
- () ->
- FetchScanTasksResponse.builder()
- .withPlanTasks(List.of("task1", "task2"))
- .withDeleteFiles(List.of(FILE_A_DELETES))
- .build())
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage(
- "Invalid response: deleteFiles should only be returned with fileScanTasks that reference them");
-
String invalidJson =
"{\"plan-tasks\":[\"task1\",\"task2\"],"
+ "\"delete-files\":[{\"spec-id\":0,\"content\":\"position-deletes\","
@@ -154,4 +144,30 @@ public class TestFetchScanTasksResponseParser {
assertThat(FetchScanTasksResponseParser.toJson(copyResponse, false)).isEqualTo(expectedToJson);
}
+
+ @Test
+ public void clearingFileScanTasksAlsoClearsDerivedDeleteFiles() {
+ ResidualEvaluator residualEvaluator =
+ ResidualEvaluator.of(SPEC, Expressions.equal("id", 1), true);
+ FileScanTask fileScanTask =
+ new BaseFileScanTask(
+ FILE_A,
+ new DeleteFile[] {FILE_A_DELETES},
+ SchemaParser.toJson(SCHEMA),
+ PartitionSpecParser.toJson(SPEC),
+ residualEvaluator);
+
+ // deleteFiles are derived from fileScanTasks, so passing null tasks must not leave the
+ // previously derived delete files behind
+ FetchScanTasksResponse response =
+ FetchScanTasksResponse.builder()
+ .withPlanTasks(List.of("plan-task"))
+ .withFileScanTasks(List.of(fileScanTask))
+ .withFileScanTasks(null)
+ .withSpecsById(PARTITION_SPECS_BY_ID)
+ .build();
+
+ assertThat(response.fileScanTasks()).isNull();
+ assertThat(response.deleteFiles()).isNull();
+ }
}
diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
index 75f7113a55..e26553d97f 100644
--- a/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
+++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestPlanTableScanResponseParser.java
@@ -199,17 +199,6 @@ public class TestPlanTableScanResponseParser {
@Test
public void roundTripSerdeWithInvalidPlanStatusSubmittedWithDeleteFilesNoFileScanTasksPresent() {
- assertThatThrownBy(
- () ->
- PlanTableScanResponse.builder()
- .withPlanStatus(PlanStatus.SUBMITTED)
- .withPlanId("somePlanId")
- .withDeleteFiles(List.of(FILE_A_DELETES))
- .build())
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessage(
- "Invalid response: deleteFiles should only be returned with fileScanTasks that reference them");
-
String invalidJson =
"{\"status\":\"submitted\","
+ "\"plan-id\":\"somePlanId\","
@@ -267,7 +256,6 @@ public class TestPlanTableScanResponseParser {
.withPlanStatus(fromResponse.planStatus())
.withPlanId(fromResponse.planId())
.withPlanTasks(fromResponse.planTasks())
- .withDeleteFiles(fromResponse.deleteFiles())
.withFileScanTasks(fromResponse.fileScanTasks())
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
@@ -477,6 +465,35 @@ public class TestPlanTableScanResponseParser {
}
}
+ @Test
+ public void toBuilderClearsDeleteFilesWhenClearingFileScanTasks() {
+ ResidualEvaluator residualEvaluator =
+ ResidualEvaluator.of(SPEC, Expressions.alwaysTrue(), true);
+ FileScanTask task =
+ new BaseFileScanTask(
+ FILE_A,
+ new DeleteFile[] {FILE_A_DELETES},
+ SchemaParser.toJson(SCHEMA),
+ PartitionSpecParser.toJson(SPEC),
+ residualEvaluator);
+
+ PlanTableScanResponse completed =
+ PlanTableScanResponse.builder()
+ .withPlanStatus(PlanStatus.COMPLETED)
+ .withFileScanTasks(List.of(task))
+ .withSpecsById(PARTITION_SPECS_BY_ID)
+ .build();
+ assertThat(completed.deleteFiles()).containsExactly(FILE_A_DELETES);
+
+ // clearing the tasks must clear the delete files derived from them, otherwise validate()
+ // rejects the copy because the retained delete files reference tasks that are no longer present
+ PlanTableScanResponse failed =
+ completed.toBuilder().withPlanStatus(PlanStatus.FAILED).withFileScanTasks(null).build();
+
+ assertThat(failed.fileScanTasks()).isNull();
+ assertThat(failed.deleteFiles()).isNull();
+ }
+
@Test
public void roundTripSerdeWithoutDeleteFiles() {
ResidualEvaluator residualEvaluator =
diff --git a/open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTServerCatalogAdapter.java b/open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTServerCatalogAdapter.java
index fde23d4b93..52dae1434f 100644
--- a/open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTServerCatalogAdapter.java
+++ b/open-api/src/testFixtures/java/org/apache/iceberg/rest/RESTServerCatalogAdapter.java
@@ -60,21 +60,13 @@ class RESTServerCatalogAdapter extends RESTCatalogAdapter {
} else if (restResponse instanceof PlanTableScanResponse response
&& PlanStatus.COMPLETED == response.planStatus()) {
return (T)
- PlanTableScanResponse.builder()
- .withPlanStatus(response.planStatus())
- .withPlanId(response.planId())
- .withFileScanTasks(response.fileScanTasks())
- .withSpecsById(response.specsById())
+ response.toBuilder()
.withCredentials(createStorageCredentials(catalogContext.configuration()))
.build();
} else if (restResponse instanceof FetchPlanningResultResponse response
&& PlanStatus.COMPLETED == response.planStatus()) {
return (T)
- FetchPlanningResultResponse.builder()
- .withPlanStatus(response.planStatus())
- .withFileScanTasks(response.fileScanTasks())
- .withPlanTasks(response.planTasks())
- .withSpecsById(response.specsById())
+ response.toBuilder()
.withCredentials(createStorageCredentials(catalogContext.configuration()))
.build();
}