gaborkaszab commented on code in PR #16125:
URL: https://github.com/apache/iceberg/pull/16125#discussion_r3636459439
##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -347,7 +357,11 @@ void update(ContentFile<?> file, Snapshot snapshot) {
break;
case POSITION_DELETES:
this.posDeleteRecordCount += file.recordCount();
- this.posDeleteFileCount += 1;
+ if (ContentFileUtil.isDV((DeleteFile) file)) {
Review Comment:
I'm wondering if it's 100% safe to do the explicit conversion here. I know
that we get here if file content is pos-dels, but does that guarantee that the
object is an instance of `DeleteFile`?
##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java:
##########
@@ -1758,6 +1769,44 @@ public void testPartitionsTableDeleteStats() {
}
}
+ @Test
+ public void testPartitionsTableDvCount() {
+ TableIdentifier tableIdentifier = TableIdentifier.of("db",
"partitions_dv_test");
+ Table table =
+ createTable(
+ tableIdentifier, SCHEMA, SPEC,
ImmutableMap.of(TableProperties.FORMAT_VERSION, "3"));
+ Dataset<Row> df1 =
+ spark.createDataFrame(Lists.newArrayList(new SimpleRecord(1, "a")),
SimpleRecord.class);
+ df1.select("id", "data")
+ .write()
+ .format("iceberg")
+ .mode("append")
+ .save(loadLocation(tableIdentifier));
+ Dataset<Row> df2 =
+ spark.createDataFrame(Lists.newArrayList(new SimpleRecord(1, "b")),
SimpleRecord.class);
+ df2.select("id", "data")
+ .write()
+ .format("iceberg")
+ .mode("append")
+ .save(loadLocation(tableIdentifier));
+ table.refresh();
+ List<DataFile> dataFiles = TestHelpers.dataFiles(table);
+ DeleteFile dvFile1 = FileGenerationUtil.generateDV(table,
dataFiles.get(0));
+ DeleteFile dvFile2 = FileGenerationUtil.generateDV(table,
dataFiles.get(1));
+ table.newRowDelta().addDeletes(dvFile1).addDeletes(dvFile2).commit();
+ table.refresh();
+ List<Row> actual =
+ spark
+ .read()
+ .format("iceberg")
+ .load(loadLocation(tableIdentifier, "partitions"))
+ .collectAsList();
+ assertThat(actual).hasSize(1);
+ assertThat((int) actual.get(0).getAs("dv_count"))
+ .as("dv_count should be 2 after adding two DVs")
Review Comment:
I see some tests use this `as()` stuff but I don't think this is needed.
--
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]
