szehon-ho commented on code in PR #7581:
URL: https://github.com/apache/iceberg/pull/7581#discussion_r1237874259
##########
spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java:
##########
@@ -1404,6 +1424,158 @@ public void testPartitionsTable() {
}
}
+ @Test
+ public void testPartitionsTableLastUpdatedSnapshot() {
+ TableIdentifier tableIdentifier = TableIdentifier.of("db",
"partitions_test");
+ Table table = createTable(tableIdentifier, SCHEMA, SPEC);
+ Table partitionsTable = loadTable(tableIdentifier, "partitions");
+ Dataset<Row> df1 =
+ spark.createDataFrame(
+ Lists.newArrayList(new SimpleRecord(1, "1"), new SimpleRecord(2,
"2")),
+ SimpleRecord.class);
+ Dataset<Row> df2 =
+ spark.createDataFrame(Lists.newArrayList(new SimpleRecord(2, "20")),
SimpleRecord.class);
+
+ df1.select("id", "data")
+ .write()
+ .format("iceberg")
+ .mode("append")
+ .save(loadLocation(tableIdentifier));
+
+ table.refresh();
+ long firstCommitId = table.currentSnapshot().snapshotId();
+
+ // add a second file
+ df2.select("id", "data")
+ .write()
+ .format("iceberg")
+ .mode("append")
+ .save(loadLocation(tableIdentifier));
+
+ table.refresh();
+ long secondCommitId = table.currentSnapshot().snapshotId();
+
+ // rewrite manifest carry over the data file to its committing snapshot
relationship
Review Comment:
Minor: How about 'check if rewrite manifest does not override metadata about
data file's creating snapshot' Is it more clear?
##########
core/src/main/java/org/apache/iceberg/PartitionsTable.java:
##########
@@ -49,6 +49,16 @@ public class PartitionsTable extends BaseMetadataTable {
new Schema(
Types.NestedField.required(1, "partition",
Partitioning.partitionType(table)),
Types.NestedField.required(4, "spec_id", Types.IntegerType.get()),
+ Types.NestedField.required(
Review Comment:
Question: did we consider making it optional, and returning null? A
precedent may be snapshots table parentId field.
##########
core/src/test/java/org/apache/iceberg/MetadataTableScanTestBase.java:
##########
@@ -81,18 +81,20 @@ protected void validateTaskScanResiduals(TableScan scan,
boolean ignoreResiduals
}
protected void validateSingleFieldPartition(
- CloseableIterable<ContentFile<?>> files, int partitionValue) {
+ CloseableIterable<ManifestEntry<? extends ContentFile<?>>> files, int
partitionValue) {
Review Comment:
Nit: do you think we can remove "extends ContentFile<?>" here as well?
Compiler seems to be able to figure it out without it.
--
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]
