This is an automated email from the ASF dual-hosted git repository.
singhpk234 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 ba28a3365a Core: Deprecate scan response builder deleteFiles API
(#14838)
ba28a3365a is described below
commit ba28a3365a1a0e3cd64875c5d76191080799e8bf
Author: Amogh Jahagirdar <[email protected]>
AuthorDate: Mon Dec 15 10:30:26 2025 -0700
Core: Deprecate scan response builder deleteFiles API (#14838)
Users should not be able to build responses by passing an explicit list of
delete files. They already have to pass through a list of file scan tasks which
contain the delete files. It's also a bit brittle of an API just because
someone could pass through file scan tasks and unrelated delete files.
This change deprecates the exposing of the deleteFiles builder API, and
will just use the file scan tasks as a source of truth.
---
.../org/apache/iceberg/rest/CatalogHandlers.java | 15 ------------
.../iceberg/rest/TableScanResponseParser.java | 10 ++++++++
.../rest/responses/BaseScanTaskResponse.java | 27 +++++++++++++++++-----
.../FetchPlanningResultResponseParser.java | 1 -
.../responses/FetchScanTasksResponseParser.java | 1 -
.../responses/PlanTableScanResponseParser.java | 1 -
.../TestFetchPlanningResultResponseParser.java | 2 --
.../TestFetchScanTasksResponseParser.java | 2 --
.../responses/TestPlanTableScanResponseParser.java | 5 ----
9 files changed, 31 insertions(+), 33 deletions(-)
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 82695eaf78..e3eff1228a 100644
--- a/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
+++ b/core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java
@@ -700,11 +700,6 @@ public class CatalogHandlers {
.withPlanStatus(PlanStatus.COMPLETED)
.withPlanId(planId)
.withFileScanTasks(initial.first())
- .withDeleteFiles(
- initial.first().stream()
- .flatMap(task -> task.deletes().stream())
- .distinct()
- .collect(Collectors.toList()))
.withSpecsById(table.specs());
if (!nextPlanTasks.isEmpty()) {
@@ -733,11 +728,6 @@ public class CatalogHandlers {
Pair<List<FileScanTask>, String> initial =
IN_MEMORY_PLANNING_STATE.initialScanTasksFor(planId);
return FetchPlanningResultResponse.builder()
.withPlanStatus(PlanStatus.COMPLETED)
- .withDeleteFiles(
- initial.first().stream()
- .flatMap(task -> task.deletes().stream())
- .distinct()
- .collect(Collectors.toList()))
.withFileScanTasks(initial.first())
.withPlanTasks(IN_MEMORY_PLANNING_STATE.nextPlanTask(initial.second()))
.withSpecsById(table.specs())
@@ -762,11 +752,6 @@ public class CatalogHandlers {
.withFileScanTasks(fileScanTasks)
.withPlanTasks(IN_MEMORY_PLANNING_STATE.nextPlanTask(planTask))
.withSpecsById(table.specs())
- .withDeleteFiles(
- fileScanTasks.stream()
- .flatMap(task -> task.deletes().stream())
- .distinct()
- .collect(Collectors.toList()))
.build();
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
b/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
index 084e74651a..02d9890494 100644
--- a/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
+++ b/core/src/main/java/org/apache/iceberg/rest/TableScanResponseParser.java
@@ -77,7 +77,17 @@ public class TableScanResponseParser {
fileScanTaskList.add(fileScanTask);
}
+ if (fileScanTaskList.isEmpty()) {
+ Preconditions.checkArgument(
+ deleteFiles == null || deleteFiles.isEmpty(),
+ "Invalid response: deleteFiles should only be returned with
fileScanTasks that reference them");
+ }
+
return fileScanTaskList;
+ } else {
+ Preconditions.checkArgument(
+ deleteFiles == null || deleteFiles.isEmpty(),
+ "Invalid response: deleteFiles should only be returned with
fileScanTasks that reference them");
}
return null;
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 f98a24cf79..e83cacc484 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
@@ -23,13 +23,15 @@ import java.util.Map;
import org.apache.iceberg.DeleteFile;
import org.apache.iceberg.FileScanTask;
import org.apache.iceberg.PartitionSpec;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.rest.RESTResponse;
+import org.apache.iceberg.util.DeleteFileSet;
public abstract class BaseScanTaskResponse implements RESTResponse {
private final List<String> planTasks;
private final List<FileScanTask> fileScanTasks;
- private final List<DeleteFile> deleteFiles;
+ private final DeleteFileSet deleteFiles;
private final Map<Integer, PartitionSpec> specsById;
protected BaseScanTaskResponse(
@@ -39,7 +41,7 @@ public abstract class BaseScanTaskResponse implements
RESTResponse {
Map<Integer, PartitionSpec> specsById) {
this.planTasks = planTasks;
this.fileScanTasks = fileScanTasks;
- this.deleteFiles = deleteFiles;
+ this.deleteFiles = deleteFiles == null ? null :
DeleteFileSet.of(deleteFiles);
this.specsById = specsById;
}
@@ -52,7 +54,7 @@ public abstract class BaseScanTaskResponse implements
RESTResponse {
}
public List<DeleteFile> deleteFiles() {
- return deleteFiles;
+ return deleteFiles == null ? null :
Lists.newArrayList(deleteFiles.iterator());
}
/**
@@ -66,7 +68,7 @@ public abstract class BaseScanTaskResponse implements
RESTResponse {
public abstract static class Builder<B extends Builder<B, R>, R extends
BaseScanTaskResponse> {
private List<String> planTasks;
private List<FileScanTask> fileScanTasks;
- private List<DeleteFile> deleteFiles;
+ private DeleteFileSet deleteFiles;
private Map<Integer, PartitionSpec> specsById;
protected Builder() {}
@@ -83,11 +85,20 @@ 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());
+ }
return self();
}
+ /**
+ * @deprecated since 1.11.0, will be removed in 1.12.0.
+ */
+ @Deprecated
public B withDeleteFiles(List<DeleteFile> deleteFilesList) {
- this.deleteFiles = deleteFilesList;
+ this.deleteFiles = DeleteFileSet.of(deleteFilesList);
return self();
}
@@ -108,8 +119,12 @@ 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() {
- return deleteFiles;
+ return deleteFiles == null ? null :
Lists.newArrayList(deleteFiles.iterator());
}
/**
diff --git
a/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponseParser.java
b/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponseParser.java
index 2ce416a0c3..5400ef1dd1 100644
---
a/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponseParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/responses/FetchPlanningResultResponseParser.java
@@ -86,7 +86,6 @@ public class FetchPlanningResultResponseParser {
.withPlanStatus(planStatus)
.withPlanTasks(planTasks)
.withFileScanTasks(fileScanTasks)
- .withDeleteFiles(deleteFiles)
.withSpecsById(specsById)
.build();
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponseParser.java
b/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponseParser.java
index 14ef77b39f..e1a4d8117c 100644
---
a/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponseParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/responses/FetchScanTasksResponseParser.java
@@ -79,7 +79,6 @@ public class FetchScanTasksResponseParser {
.withPlanTasks(planTasks)
.withSpecsById(specsById)
.withFileScanTasks(fileScanTasks)
- .withDeleteFiles(deleteFiles)
.build();
}
}
diff --git
a/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponseParser.java
b/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponseParser.java
index df2ad94a32..c2f47b86d3 100644
---
a/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponseParser.java
+++
b/core/src/main/java/org/apache/iceberg/rest/responses/PlanTableScanResponseParser.java
@@ -110,7 +110,6 @@ public class PlanTableScanResponseParser {
.withPlanStatus(planStatus)
.withPlanTasks(planTasks)
.withFileScanTasks(fileScanTasks)
- .withDeleteFiles(deleteFiles)
.withSpecsById(specsById);
if (json.hasNonNull(STORAGE_CREDENTIALS)) {
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 7c1021d4af..5d9d2cad82 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
@@ -186,7 +186,6 @@ public class TestFetchPlanningResultResponseParser {
FetchPlanningResultResponse.builder()
.withPlanStatus(planStatus)
.withFileScanTasks(List.of(fileScanTask))
- .withDeleteFiles(List.of(FILE_A_DELETES))
// assume this has been set
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
@@ -219,7 +218,6 @@ public class TestFetchPlanningResultResponseParser {
FetchPlanningResultResponse.builder()
.withPlanStatus(fromResponse.planStatus())
.withPlanTasks(fromResponse.planTasks())
- .withDeleteFiles(fromResponse.deleteFiles())
.withFileScanTasks(fromResponse.fileScanTasks())
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
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 d7824bc6a6..7b44b3533a 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
@@ -122,7 +122,6 @@ public class TestFetchScanTasksResponseParser {
FetchScanTasksResponse response =
FetchScanTasksResponse.builder()
.withFileScanTasks(List.of(fileScanTask))
- .withDeleteFiles(List.of(FILE_A_DELETES))
// assume you have set this already
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
@@ -149,7 +148,6 @@ public class TestFetchScanTasksResponseParser {
FetchScanTasksResponse copyResponse =
FetchScanTasksResponse.builder()
.withPlanTasks(fromResponse.planTasks())
- .withDeleteFiles(fromResponse.deleteFiles())
.withFileScanTasks(fromResponse.fileScanTasks())
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
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 e2c9f21dab..454e838bcc 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
@@ -238,7 +238,6 @@ public class TestPlanTableScanResponseParser {
PlanTableScanResponse.builder()
.withPlanStatus(PlanStatus.COMPLETED)
.withFileScanTasks(List.of(fileScanTask))
- .withDeleteFiles(List.of(FILE_A_DELETES))
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
@@ -307,7 +306,6 @@ public class TestPlanTableScanResponseParser {
PlanTableScanResponse.builder()
.withPlanStatus(PlanStatus.COMPLETED)
.withFileScanTasks(List.of(taskA, taskB, taskC))
- .withDeleteFiles(List.of(FILE_A_DELETES, FILE_B_DELETES,
FILE_C2_DELETES))
.withSpecsById(PARTITION_SPECS_BY_ID)
.build();
@@ -425,7 +423,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();
@@ -578,7 +575,6 @@ public class TestPlanTableScanResponseParser {
PlanTableScanResponse.builder()
.withPlanStatus(PlanStatus.COMPLETED)
.withFileScanTasks(List.of(fileScanTask))
- .withDeleteFiles(List.of(FILE_A_DELETES))
.withSpecsById(PARTITION_SPECS_BY_ID)
.withCredentials(credentials)
.build();
@@ -645,7 +641,6 @@ public class TestPlanTableScanResponseParser {
.withPlanStatus(fromResponse.planStatus())
.withPlanId(fromResponse.planId())
.withPlanTasks(fromResponse.planTasks())
- .withDeleteFiles(fromResponse.deleteFiles())
.withFileScanTasks(fromResponse.fileScanTasks())
.withSpecsById(PARTITION_SPECS_BY_ID)
.withCredentials(credentials)
