rdblue commented on a change in pull request #1497:
URL: https://github.com/apache/iceberg/pull/1497#discussion_r493954344



##########
File path: 
spark/src/test/java/org/apache/iceberg/spark/source/TestSparkReaderDeletes.java
##########
@@ -93,212 +102,4 @@ public void createTable() throws IOException {
   public void dropTable() {
     catalog.dropTable(TableIdentifier.of("default", "table"));
   }
-
-  @Test
-  public void testEqualityDeletes() throws IOException {
-    Schema deleteRowSchema = table.schema().select("data");
-    Record dataDelete = GenericRecord.create(deleteRowSchema);
-    List<Record> dataDeletes = Lists.newArrayList(
-        dataDelete.copy("data", "a"), // id = 29
-        dataDelete.copy("data", "d"), // id = 89
-        dataDelete.copy("data", "g") // id = 122
-    );
-
-    DeleteFile eqDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), dataDeletes, 
deleteRowSchema);
-
-    table.newRowDelta()
-        .addDeletes(eqDeletes)
-        .commit();
-
-    StructLikeSet expected = rowSetWithoutIds(29, 89, 122);
-    StructLikeSet actual = rowSet(table);
-
-    Assert.assertEquals("Table should contain expected rows", expected, 
actual);
-  }
-
-  @Test
-  public void testEqualityDeletesWithRequiredEqColumn() throws IOException {
-    Schema deleteRowSchema = table.schema().select("data");
-    Record dataDelete = GenericRecord.create(deleteRowSchema);
-    List<Record> dataDeletes = Lists.newArrayList(
-        dataDelete.copy("data", "a"), // id = 29
-        dataDelete.copy("data", "d"), // id = 89
-        dataDelete.copy("data", "g") // id = 122
-    );
-
-    DeleteFile eqDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), dataDeletes, 
deleteRowSchema);
-
-    table.newRowDelta()
-        .addDeletes(eqDeletes)
-        .commit();
-
-    StructLikeSet expected = selectColumns(rowSetWithoutIds(29, 89, 122), 
"id");
-    StructLikeSet actual = rowSet(table, "id"); // data is added by the reader 
to apply the eq deletes
-
-    Assert.assertEquals("Table should contain expected rows", expected, 
actual);
-  }
-
-  @Test
-  public void testPositionDeletes() throws IOException {
-    List<Pair<CharSequence, Long>> deletes = Lists.newArrayList(
-        Pair.of(dataFile.path(), 0L), // id = 29
-        Pair.of(dataFile.path(), 3L), // id = 89
-        Pair.of(dataFile.path(), 6L) // id = 122
-    );
-
-    DeleteFile posDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), deletes);
-
-    table.newRowDelta()
-        .addDeletes(posDeletes)
-        .commit();
-
-    StructLikeSet expected = rowSetWithoutIds(29, 89, 122);
-    StructLikeSet actual = rowSet(table);
-
-    Assert.assertEquals("Table should contain expected rows", expected, 
actual);
-  }
-
-  @Test
-  public void testMixedPositionAndEqualityDeletes() throws IOException {
-    Schema dataSchema = table.schema().select("data");
-    Record dataDelete = GenericRecord.create(dataSchema);
-    List<Record> dataDeletes = Lists.newArrayList(
-        dataDelete.copy("data", "a"), // id = 29
-        dataDelete.copy("data", "d"), // id = 89
-        dataDelete.copy("data", "g") // id = 122
-    );
-
-    DeleteFile eqDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), dataDeletes, 
dataSchema);
-
-    List<Pair<CharSequence, Long>> deletes = Lists.newArrayList(
-        Pair.of(dataFile.path(), 3L), // id = 89
-        Pair.of(dataFile.path(), 5L) // id = 121
-    );
-
-    DeleteFile posDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), deletes);
-
-    table.newRowDelta()
-        .addDeletes(eqDeletes)
-        .addDeletes(posDeletes)
-        .commit();
-
-    StructLikeSet expected = rowSetWithoutIds(29, 89, 121, 122);
-    StructLikeSet actual = rowSet(table);
-
-    Assert.assertEquals("Table should contain expected rows", expected, 
actual);
-  }
-
-  @Test
-  public void testMultipleEqualityDeleteSchemas() throws IOException {
-    Schema dataSchema = table.schema().select("data");
-    Record dataDelete = GenericRecord.create(dataSchema);
-    List<Record> dataDeletes = Lists.newArrayList(
-        dataDelete.copy("data", "a"), // id = 29
-        dataDelete.copy("data", "d"), // id = 89
-        dataDelete.copy("data", "g") // id = 122
-    );
-
-    DeleteFile dataEqDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), dataDeletes, 
dataSchema);
-
-    Schema idSchema = table.schema().select("id");
-    Record idDelete = GenericRecord.create(idSchema);
-    List<Record> idDeletes = Lists.newArrayList(
-        idDelete.copy("id", 121), // id = 121
-        idDelete.copy("id", 29) // id = 29
-    );
-
-    DeleteFile idEqDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), idDeletes, 
idSchema);
-
-    table.newRowDelta()
-        .addDeletes(dataEqDeletes)
-        .addDeletes(idEqDeletes)
-        .commit();
-
-    StructLikeSet expected = rowSetWithoutIds(29, 89, 121, 122);
-    StructLikeSet actual = rowSet(table);
-
-    Assert.assertEquals("Table should contain expected rows", expected, 
actual);
-  }
-
-  @Test
-  public void testEqualityDeleteByNull() throws IOException {
-    // data is required in the test table; make it optional for this test
-    table.updateSchema()
-        .makeColumnOptional("data")
-        .commit();
-
-    // add a new data file with a record where data is null
-    Record record = GenericRecord.create(table.schema());
-    DataFile dataFileWithNull = FileHelpers.writeDataFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0),
-        Lists.newArrayList(record.copy("id", 131, "data", null)));
-
-    table.newAppend()
-        .appendFile(dataFileWithNull)
-        .commit();
-
-    // delete where data is null
-    Schema dataSchema = table.schema().select("data");
-    Record dataDelete = GenericRecord.create(dataSchema);
-    List<Record> dataDeletes = Lists.newArrayList(
-        dataDelete.copy("data", null) // id = 131
-    );
-
-    DeleteFile eqDeletes = FileHelpers.writeDeleteFile(
-        table, Files.localOutput(temp.newFile()), Row.of(0), dataDeletes, 
dataSchema);
-
-    table.newRowDelta()
-        .addDeletes(eqDeletes)
-        .commit();
-
-    StructLikeSet expected = rowSetWithoutIds(131);
-    StructLikeSet actual = rowSet(table);
-
-    Assert.assertEquals("Table should contain expected rows", expected, 
actual);
-  }
-
-  private static StructLikeSet rowSet(Table table) {
-    return rowSet(table, "*");
-  }
-
-  private static StructLikeSet rowSet(Table table, String... columns) {

Review comment:
       This method is what reads the rows from the table using Spark. Deleting 
this method and using the one in `DeletesReadTest` makes this test suite use 
the exact same read path as the generics -- `IcebergGenerics`.
   
   You can probably make this method abstract and implement it in both classes 
to get around this. You'll also need to implement a read using the input format 
or Hive runner to test the Hive code.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to