rdblue commented on code in PR #6965:
URL: https://github.com/apache/iceberg/pull/6965#discussion_r1132987335
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java:
##########
@@ -139,105 +143,121 @@ public void testDeleteFileThenMetadataDelete() throws
Exception {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware'), (null,
'hr')", tableName);
+ createBranch();
// MOR mode: writes a delete file as null cannot be deleted by metadata
- sql("DELETE FROM %s AS t WHERE t.id IS NULL", tableName);
+ sql("DELETE FROM %s AS t WHERE t.id IS NULL", commitTarget());
// Metadata Delete
Table table = Spark3Util.loadIcebergTable(spark, tableName);
- Set<DataFile> dataFilesBefore = TestHelpers.dataFiles(table);
+ Set<DataFile> dataFilesBefore = TestHelpers.dataFiles(table, branch);
- sql("DELETE FROM %s AS t WHERE t.id = 1", tableName);
+ sql("DELETE FROM %s AS t WHERE t.id = 1", commitTarget());
- Set<DataFile> dataFilesAfter = TestHelpers.dataFiles(table);
+ Set<DataFile> dataFilesAfter = TestHelpers.dataFiles(table, branch);
Assert.assertTrue(
"Data file should have been removed", dataFilesBefore.size() >
dataFilesAfter.size());
assertEquals(
"Should have expected rows",
ImmutableList.of(row(2, "hardware")),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
}
@Test
public void testDeleteWithFalseCondition() {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware')", tableName);
+ createBranch();
- sql("DELETE FROM %s WHERE id = 1 AND id > 20", tableName);
+ sql("DELETE FROM %s WHERE id = 1 AND id > 20", commitTarget());
Table table = validationCatalog.loadTable(tableIdent);
Assert.assertEquals("Should have 2 snapshots", 2,
Iterables.size(table.snapshots()));
assertEquals(
"Should have expected rows",
ImmutableList.of(row(1, "hr"), row(2, "hardware")),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
}
@Test
public void testDeleteFromEmptyTable() {
+ Assume.assumeFalse("Custom branch does not exist for empty table",
"test".equals(branch));
createAndInitUnpartitionedTable();
- sql("DELETE FROM %s WHERE id IN (1)", tableName);
- sql("DELETE FROM %s WHERE dep = 'hr'", tableName);
+ sql("DELETE FROM %s WHERE id IN (1)", commitTarget());
+ sql("DELETE FROM %s WHERE dep = 'hr'", commitTarget());
Table table = validationCatalog.loadTable(tableIdent);
Assert.assertEquals("Should have 2 snapshots", 2,
Iterables.size(table.snapshots()));
assertEquals(
"Should have expected rows",
ImmutableList.of(),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
+ }
+
+ @Test
+ public void testDeleteFromNonExistingCustomBranch() {
+ Assume.assumeTrue("Test only applicable to custom branch",
"test".equals(branch));
+ createAndInitUnpartitionedTable();
+
+ Assertions.assertThatThrownBy(() -> sql("DELETE FROM %s WHERE id IN (1)",
commitTarget()))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Cannot operate against non-existing branch: test");
}
@Test
public void testExplain() {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware'), (null,
'hr')", tableName);
+ createBranch();
- sql("EXPLAIN DELETE FROM %s WHERE id <=> 1", tableName);
+ sql("EXPLAIN DELETE FROM %s WHERE id <=> 1", commitTarget());
- sql("EXPLAIN DELETE FROM %s WHERE true", tableName);
+ sql("EXPLAIN DELETE FROM %s WHERE true", commitTarget());
Table table = validationCatalog.loadTable(tableIdent);
Assert.assertEquals("Should have 1 snapshot", 1,
Iterables.size(table.snapshots()));
assertEquals(
"Should have expected rows",
ImmutableList.of(row(1, "hr"), row(2, "hardware"), row(null, "hr")),
- sql("SELECT * FROM %s ORDER BY id ASC NULLS LAST", tableName));
+ sql("SELECT * FROM %s ORDER BY id ASC NULLS LAST", commitTarget()));
}
@Test
public void testDeleteWithAlias() {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware'), (null,
'hr')", tableName);
+ createBranch();
- sql("DELETE FROM %s AS t WHERE t.id IS NULL", tableName);
+ sql("DELETE FROM %s AS t WHERE t.id IS NULL", commitTarget());
assertEquals(
"Should have expected rows",
ImmutableList.of(row(1, "hr"), row(2, "hardware")),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
}
@Test
public void testDeleteWithDynamicFileFiltering() throws NoSuchTableException
{
createAndInitPartitionedTable();
- append(new Employee(1, "hr"), new Employee(3, "hr"));
+ append(tableName, new Employee(1, "hr"), new Employee(3, "hr"));
+ createBranch();
Review Comment:
Is it worth supporting `createBranch` when there's no table data? We could
create an empty snapshot and branch automatically.
##########
spark/v3.3/spark-extensions/src/test/java/org/apache/iceberg/spark/extensions/TestDelete.java:
##########
@@ -139,105 +143,121 @@ public void testDeleteFileThenMetadataDelete() throws
Exception {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware'), (null,
'hr')", tableName);
+ createBranch();
// MOR mode: writes a delete file as null cannot be deleted by metadata
- sql("DELETE FROM %s AS t WHERE t.id IS NULL", tableName);
+ sql("DELETE FROM %s AS t WHERE t.id IS NULL", commitTarget());
// Metadata Delete
Table table = Spark3Util.loadIcebergTable(spark, tableName);
- Set<DataFile> dataFilesBefore = TestHelpers.dataFiles(table);
+ Set<DataFile> dataFilesBefore = TestHelpers.dataFiles(table, branch);
- sql("DELETE FROM %s AS t WHERE t.id = 1", tableName);
+ sql("DELETE FROM %s AS t WHERE t.id = 1", commitTarget());
- Set<DataFile> dataFilesAfter = TestHelpers.dataFiles(table);
+ Set<DataFile> dataFilesAfter = TestHelpers.dataFiles(table, branch);
Assert.assertTrue(
"Data file should have been removed", dataFilesBefore.size() >
dataFilesAfter.size());
assertEquals(
"Should have expected rows",
ImmutableList.of(row(2, "hardware")),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
}
@Test
public void testDeleteWithFalseCondition() {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware')", tableName);
+ createBranch();
- sql("DELETE FROM %s WHERE id = 1 AND id > 20", tableName);
+ sql("DELETE FROM %s WHERE id = 1 AND id > 20", commitTarget());
Table table = validationCatalog.loadTable(tableIdent);
Assert.assertEquals("Should have 2 snapshots", 2,
Iterables.size(table.snapshots()));
assertEquals(
"Should have expected rows",
ImmutableList.of(row(1, "hr"), row(2, "hardware")),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
}
@Test
public void testDeleteFromEmptyTable() {
+ Assume.assumeFalse("Custom branch does not exist for empty table",
"test".equals(branch));
createAndInitUnpartitionedTable();
- sql("DELETE FROM %s WHERE id IN (1)", tableName);
- sql("DELETE FROM %s WHERE dep = 'hr'", tableName);
+ sql("DELETE FROM %s WHERE id IN (1)", commitTarget());
+ sql("DELETE FROM %s WHERE dep = 'hr'", commitTarget());
Table table = validationCatalog.loadTable(tableIdent);
Assert.assertEquals("Should have 2 snapshots", 2,
Iterables.size(table.snapshots()));
assertEquals(
"Should have expected rows",
ImmutableList.of(),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
+ }
+
+ @Test
+ public void testDeleteFromNonExistingCustomBranch() {
+ Assume.assumeTrue("Test only applicable to custom branch",
"test".equals(branch));
+ createAndInitUnpartitionedTable();
+
+ Assertions.assertThatThrownBy(() -> sql("DELETE FROM %s WHERE id IN (1)",
commitTarget()))
+ .isInstanceOf(ValidationException.class)
+ .hasMessage("Cannot operate against non-existing branch: test");
}
@Test
public void testExplain() {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware'), (null,
'hr')", tableName);
+ createBranch();
- sql("EXPLAIN DELETE FROM %s WHERE id <=> 1", tableName);
+ sql("EXPLAIN DELETE FROM %s WHERE id <=> 1", commitTarget());
- sql("EXPLAIN DELETE FROM %s WHERE true", tableName);
+ sql("EXPLAIN DELETE FROM %s WHERE true", commitTarget());
Table table = validationCatalog.loadTable(tableIdent);
Assert.assertEquals("Should have 1 snapshot", 1,
Iterables.size(table.snapshots()));
assertEquals(
"Should have expected rows",
ImmutableList.of(row(1, "hr"), row(2, "hardware"), row(null, "hr")),
- sql("SELECT * FROM %s ORDER BY id ASC NULLS LAST", tableName));
+ sql("SELECT * FROM %s ORDER BY id ASC NULLS LAST", commitTarget()));
}
@Test
public void testDeleteWithAlias() {
createAndInitUnpartitionedTable();
sql("INSERT INTO TABLE %s VALUES (1, 'hr'), (2, 'hardware'), (null,
'hr')", tableName);
+ createBranch();
- sql("DELETE FROM %s AS t WHERE t.id IS NULL", tableName);
+ sql("DELETE FROM %s AS t WHERE t.id IS NULL", commitTarget());
assertEquals(
"Should have expected rows",
ImmutableList.of(row(1, "hr"), row(2, "hardware")),
- sql("SELECT * FROM %s ORDER BY id", tableName));
+ sql("SELECT * FROM %s ORDER BY id", selectTarget()));
}
@Test
public void testDeleteWithDynamicFileFiltering() throws NoSuchTableException
{
createAndInitPartitionedTable();
- append(new Employee(1, "hr"), new Employee(3, "hr"));
+ append(tableName, new Employee(1, "hr"), new Employee(3, "hr"));
+ createBranch();
Review Comment:
Is it worth supporting `createBranch` when there's no table data? We could
create an empty snapshot to make it work.
--
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]
