nastra commented on code in PR #13400:
URL: https://github.com/apache/iceberg/pull/13400#discussion_r2538949372
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3163,4 +3382,459 @@ private static List<HTTPRequest>
allRequests(RESTCatalogAdapter adapter) {
verify(adapter, atLeastOnce()).execute(captor.capture(), any(), any(),
any());
return captor.getAllValues();
}
+
+ @Test
+ public void testCancelPlanWithNoActivePlan() {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+ RESTTableScan restTableScan = restTableScanFor("cancel_test_table");
+
+ // Calling cancel with no active plan should return false
+ assertThat(restTableScan.cancelPlan()).isFalse();
+ }
+
+ @Test
+ public void testCancelPlanEndpointSupport() {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+ RESTTableScan restTableScan = restTableScanFor("cancel_support_table");
+
+ // Test that cancelPlan method is available and returns false when no plan
is active
+ assertThat(restTableScan.cancelPlan()).isFalse();
+ }
+
+ @Test
+ public void testCancelPlanMethodAvailability() {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+ RESTTableScan restTableScan = restTableScanFor("cancel_method_table");
+
+ // Test that cancelPlan method is available and callable
+ // When no plan is active, it should return false
+ assertThat(restTableScan.cancelPlan()).isFalse();
+
+ // Verify the method exists and doesn't throw exceptions when called
multiple times
+ assertThat(restTableScan.cancelPlan()).isFalse();
+ }
+
+ @Test
+ public void testCancelPlanEndpointPath() {
+ TableIdentifier tableId = TableIdentifier.of("test_namespace",
"test_table");
+ String planId = "plan-abc-123";
+ ResourcePaths paths = new ResourcePaths("test-prefix");
+
+ // Test that the cancel plan path is generated correctly
+ String cancelPath = paths.plan(tableId, planId);
+
+ assertThat(cancelPath)
+
.isEqualTo("v1/test-prefix/namespaces/test_namespace/tables/test_table/plan/plan-abc-123");
+
+ // Test with different identifiers
+ TableIdentifier complexId = TableIdentifier.of(Namespace.of("db",
"schema"), "my_table");
+ String complexPath = paths.plan(complexId, "plan-xyz-789");
+
+ assertThat(complexPath).contains("/plan/plan-xyz-789");
+ assertThat(complexPath).contains("db%1Fschema"); // URL encoded namespace
separator
+ }
+
+ @Test
+ public void testIteratorCloseTriggersCancel() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+ Table table = createTableWithScanPlanning("iterator_close_table");
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+ RESTTable restTable = (RESTTable) table;
+
+ TableScan scan = restTable.newScan();
+ assertThat(scan).isInstanceOf(RESTTableScan.class);
+ boolean cancelled = isCancelled((RESTTableScan) scan);
+ assertThat(cancelled).isFalse(); // No active plan to cancel
+ }
+
+ private static boolean isCancelled(RESTTableScan scan) throws IOException {
+
+ // Get the iterable and iterator
+ CloseableIterable<FileScanTask> iterable = scan.planFiles();
+ CloseableIterator<FileScanTask> iterator = iterable.iterator();
+
+ // Verify we can close the iterator without exceptions
+ // The cancellation callback will be called (though no active plan exists)
+ iterator.close();
+
+ // Verify we can still call cancelPlan on the scan
+ return scan.cancelPlan();
+ }
+
+ @Test
+ public void testMetadataTablesWithRemotePlanning() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+ Table table = createTableWithScanPlanning("metadata_tables_test");
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+
+ // Test that metadata tables work with remote planning
+ // Files metadata table
+ Table filesTable =
+ MetadataTableUtils.createMetadataTableInstance(table,
MetadataTableType.FILES);
+ assertThat(filesTable).isNotNull();
+
+ TableScan filesScan = filesTable.newScan();
+ assertThat(filesScan).isNotNull();
+
+ // Verify metadata table scan works (should not use REST scan planning)
+ CloseableIterable<FileScanTask> filesIterable = filesScan.planFiles();
+ List<FileScanTask> filesTasks = Lists.newArrayList(filesIterable);
+ assertThat(filesTasks).isNotEmpty();
+
+ // Snapshots metadata table
+ Table snapshotsTable =
+ MetadataTableUtils.createMetadataTableInstance(table,
MetadataTableType.SNAPSHOTS);
+ assertThat(snapshotsTable).isNotNull();
+
+ TableScan snapshotsScan = snapshotsTable.newScan();
+ CloseableIterable<FileScanTask> snapshotsIterable =
snapshotsScan.planFiles();
+ List<FileScanTask> snapshotsTasks = Lists.newArrayList(snapshotsIterable);
+ assertThat(snapshotsTasks).isNotEmpty();
+
+ // Manifests metadata table
+ Table manifestsTable =
+ MetadataTableUtils.createMetadataTableInstance(table,
MetadataTableType.MANIFESTS);
+ assertThat(manifestsTable).isNotNull();
+
+ TableScan manifestsScan = manifestsTable.newScan();
+ CloseableIterable<FileScanTask> manifestsIterable =
manifestsScan.planFiles();
+ List<FileScanTask> manifestsTasks = Lists.newArrayList(manifestsIterable);
+ assertThat(manifestsTasks).isNotEmpty();
+ }
+
+ @Test
+ public void testIterableCloseTriggersCancel() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithPagination);
+ Table table = createTableWithScanPlanning("iterable_close_test");
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+ RESTTable restTable = (RESTTable) table;
+
+ TableScan scan = restTable.newScan();
+ assertThat(scan).isInstanceOf(RESTTableScan.class);
+ RESTTableScan restTableScan = (RESTTableScan) scan;
+
+ // Get the iterable
+ CloseableIterable<FileScanTask> iterable = restTableScan.planFiles();
+
+ // Verify we can close the iterable without exceptions
+ // This tests that cancellation callbacks are properly wired through
+ iterable.close();
+
+ // Verify the scan is still functional
+ boolean cancelled = restTableScan.cancelPlan();
+ assertThat(cancelled).isFalse(); // No active plan to cancel
+ }
+
+ @Test
+ public void testRESTScanPlanningWithPositionDeletes() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+ Table table = createTableWithScanPlanning("position_deletes_test");
+
+ // Add position deletes that correspond to FILE_A (which was added in
table creation)
+ table.newRowDelta().addDeletes(FILE_A_DELETES).commit();
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+
+ // Execute scan planning - should handle position deletes correctly
+ try (CloseableIterable<FileScanTask> iterable =
table.newScan().planFiles()) {
+ List<FileScanTask> tasks = Lists.newArrayList(iterable);
+
+ // Verify we get tasks back (specific count depends on implementation)
+ assertThat(tasks).hasSize(1); // 1 data file: FILE_A
+
+ // Verify specific task content and delete file associations
+ FileScanTask taskWithDeletes =
+ tasks.stream()
+ .filter(task -> !task.deletes().isEmpty())
+ .findFirst()
+ .orElseThrow(
+ () -> new AssertionError("Expected at least one task with
delete files"));
+
+ assertThat(taskWithDeletes.file().path()).isEqualTo(FILE_A.path());
+ assertThat(taskWithDeletes.deletes()).hasSize(1); // 1 delete file:
FILE_A_DELETES
+
assertThat(taskWithDeletes.deletes().get(0).path()).isEqualTo(FILE_A_DELETES.path());
+ }
+ }
+
+ @Test
+ public void testRESTScanPlanningWithEqualityDeletes() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+ Table table = createTableWithScanPlanning("equality_deletes_test");
+
+ // Add equality deletes that correspond to FILE_A
+ table.newRowDelta().addDeletes(FILE_A_EQUALITY_DELETES).commit();
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+
+ // Execute scan planning - should handle equality deletes correctly
+ try (CloseableIterable<FileScanTask> iterable =
table.newScan().planFiles()) {
+ List<FileScanTask> tasks = Lists.newArrayList(iterable);
+
+ // Verify the task count and file paths
+ assertThat(tasks).hasSize(1); // 1 data file: FILE_A
+
+ // Verify specific task content and equality delete file associations
+ FileScanTask taskWithDeletes =
+ tasks.stream()
+ .filter(task -> !task.deletes().isEmpty())
+ .findFirst()
+ .orElseThrow(
+ () -> new AssertionError("Expected at least one task with
delete files"));
+
+ assertThat(taskWithDeletes.file().path()).isEqualTo(FILE_A.path());
+ assertThat(taskWithDeletes.deletes()).hasSize(1); // 1 delete file:
FILE_A_EQUALITY_DELETES
+
assertThat(taskWithDeletes.deletes().get(0).path()).isEqualTo(FILE_A_EQUALITY_DELETES.path());
+ }
+ }
+
+ @Test
+ public void testRESTScanPlanningWithMixedDeletes() throws IOException {
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+ Table table = createTableWithScanPlanning("mixed_deletes_test");
+
+ // Add both position and equality deletes in separate commits
+ table.newRowDelta().addDeletes(FILE_A_DELETES).commit(); // Position
deletes for FILE_A
+ table
+ .newRowDelta()
+ .addDeletes(FILE_B_EQUALITY_DELETES)
+ .commit(); // Equality deletes for different partition
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+
+ // Execute scan planning - should handle mixed delete types correctly
+ try (CloseableIterable<FileScanTask> iterable =
table.newScan().planFiles()) {
+ List<FileScanTask> tasks = Lists.newArrayList(iterable);
+
+ // Verify task count: FILE_A only (FILE_B_EQUALITY_DELETES is in
different partition)
+ assertThat(tasks).hasSize(1); // 1 data file: FILE_A
+
+ // Verify FILE_A with position deletes (FILE_B_EQUALITY_DELETES not
associated since no
+ // FILE_B)
+ FileScanTask fileATask =
+ tasks.stream()
+ .filter(task -> task.file().path().equals(FILE_A.path()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Expected FILE_A in scan
tasks"));
+
+ assertThat(fileATask.deletes())
+ .hasSize(1); // 1 delete file: FILE_A_DELETES
(FILE_B_EQUALITY_DELETES not matched)
+
assertThat(fileATask.deletes().get(0).path()).isEqualTo(FILE_A_DELETES.path());
+ }
+ }
+
+ @Test
+ public void testRESTScanPlanningWithMultipleDeleteFiles() throws IOException
{
+
configurePlanningBehavior(TestPlanningBehavior.Builder::synchronousWithAllTasks);
+ Table table = createTableWithScanPlanning("multiple_deletes_test");
+
+ // Add FILE_B and FILE_C to the table (FILE_A is already added during
table creation)
+ table.newAppend().appendFile(FILE_B).appendFile(FILE_C).commit();
+
+ // Add multiple delete files corresponding to FILE_A, FILE_B, FILE_C
+ table
+ .newRowDelta()
+ .addDeletes(FILE_A_DELETES) // Position delete for FILE_A
+ .addDeletes(FILE_B_DELETES) // Position delete for FILE_B
+ .addDeletes(FILE_C_EQUALITY_DELETES) // Equality delete for FILE_C
+ .commit();
+
+ // Ensure we have a RESTTable with server-side planning enabled
+ assertThat(table).isInstanceOf(RESTTable.class);
+
+ // Execute scan planning with multiple delete files
+ try (CloseableIterable<FileScanTask> iterable =
table.newScan().planFiles()) {
+ List<FileScanTask> tasks = Lists.newArrayList(iterable);
+
+ // Verify we get tasks back (should have 3 data files: FILE_A, FILE_B,
FILE_C)
+ assertThat(tasks).hasSize(3); // 3 data files
+
+ // Verify FILE_A with position deletes
+ FileScanTask fileATask =
+ tasks.stream()
+ .filter(task -> task.file().path().equals(FILE_A.path()))
+ .findFirst()
+ .orElseThrow(() -> new AssertionError("Expected FILE_A in scan
tasks"));
Review Comment:
same as above
--
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]
