nastra commented on code in PR #14944:
URL: https://github.com/apache/iceberg/pull/14944#discussion_r2668121119
##########
core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java:
##########
@@ -3528,4 +3530,201 @@ private static List<HTTPRequest>
allRequests(RESTCatalogAdapter adapter) {
verify(adapter, atLeastOnce()).execute(captor.capture(), any(), any(),
any());
return captor.getAllValues();
}
+
+ @Test
+ public void testSerializableTableWithRESTCatalog() throws IOException,
ClassNotFoundException {
+ // Create a table with data through REST catalog
+ Schema schema =
+ new Schema(
+ required(1, "id", Types.IntegerType.get()),
+ required(2, "data", Types.StringType.get()));
+
+ TableIdentifier ident = TableIdentifier.of(Namespace.of("ns"), "table");
+ restCatalog.createNamespace(ident.namespace());
+ Table table = restCatalog.createTable(ident, schema);
+
+ // Add data files to create snapshots
+ table.newAppend().appendFile(FILE_A).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+
+ // Create SerializableTable from REST catalog
+ SerializableTable serializableTable = (SerializableTable)
SerializableTable.copyOf(table);
+
+ // Test with Java serialization
+ Table javaSerialized = TestHelpers.roundTripSerialize(serializableTable);
+ verifySerializedTable(javaSerialized, table, 2);
+
+ // Test with Kryo serialization
+ Table kryoSerialized =
TestHelpers.KryoHelpers.roundTripSerialize(serializableTable);
+ verifySerializedTable(kryoSerialized, table, 2);
+ }
+
+ private void verifySerializedTable(
+ Table deserialized, Table original, int expectedSnapshotCount) {
+ // Verify that basic operations work using serialized table metadata
+ // (tables requiring remote scan planning serialize full metadata)
+
assertThat(deserialized.schema().asStruct()).isEqualTo(original.schema().asStruct());
+
assertThat(deserialized.spec().specId()).isEqualTo(original.spec().specId());
+ assertThat(deserialized.location()).isEqualTo(original.location());
+
assertThat(deserialized.properties()).containsAllEntriesOf(original.properties());
+
+ // Verify snapshot operations work using serialized metadata
+ assertThat(deserialized.currentSnapshot()).isNotNull();
+ assertThat(deserialized.currentSnapshot().snapshotId())
+ .isEqualTo(original.currentSnapshot().snapshotId());
+
+ // Verify snapshots are accessible
+ assertThat(deserialized.snapshots()).isNotNull();
+ int snapshotCount = 0;
+ for (Snapshot snapshot : deserialized.snapshots()) {
+ snapshotCount++;
+ assertThat(snapshot).isNotNull();
+ }
+ assertThat(snapshotCount).isEqualTo(expectedSnapshotCount);
+
+ // Verify scan operations work
+ assertThat(deserialized.newScan()).isNotNull();
+ }
+
+ @Test
+ public void testSerializableTableWithRESTCatalogAndSchemaEvolution()
+ throws IOException, ClassNotFoundException {
+ // Create initial table
+ Schema initialSchema =
+ new Schema(
+ required(1, "id", Types.IntegerType.get()),
+ required(2, "data", Types.StringType.get()));
+
+ TableIdentifier ident = TableIdentifier.of(Namespace.of("ns"),
"schema_evolution_table");
+ restCatalog.createNamespace(ident.namespace());
+ Table table = restCatalog.createTable(ident, initialSchema);
+
+ // Evolve schema
+ table.updateSchema().addColumn("new_col1",
Types.IntegerType.get()).commit();
+ table.newAppend().appendFile(FILE_A).commit();
+
+ // Evolve schema again
+ table.updateSchema().addColumn("new_col2",
Types.StringType.get()).commit();
+ table.newAppend().appendFile(FILE_B).commit();
+
+ // Create and serialize table
+ SerializableTable serializableTable = (SerializableTable)
SerializableTable.copyOf(table);
+
+ // Test with Java serialization
+ Table javaSerialized = TestHelpers.roundTripSerialize(serializableTable);
+ verifySchemaEvolution(javaSerialized, table);
+
+ // Test with Kryo serialization
+ Table kryoSerialized =
TestHelpers.KryoHelpers.roundTripSerialize(serializableTable);
+ verifySchemaEvolution(kryoSerialized, table);
+ }
+
+ private void verifySchemaEvolution(Table deserialized, Table original) {
+ // Verify current schema
+
assertThat(deserialized.schema().columns()).hasSize(original.schema().columns().size());
+
+ // Verify all historical schemas are preserved
+ assertThat(deserialized.schemas()).isNotNull();
+
assertThat(deserialized.schemas().size()).isEqualTo(original.schemas().size());
+
+ // Verify scans work
+ assertThat(deserialized.newScan()).isNotNull();
+ }
+
+ @Test
+ public void testSerializableTableWithRESTCatalogAndSnapshotRefs()
+ throws IOException, ClassNotFoundException {
+ // Create table with data
+ Schema schema =
+ new Schema(
+ required(1, "id", Types.IntegerType.get()),
+ required(2, "data", Types.StringType.get()));
+
+ TableIdentifier ident = TableIdentifier.of(Namespace.of("ns"),
"refs_table");
+ restCatalog.createNamespace(ident.namespace());
+ Table table = restCatalog.createTable(ident, schema);
+
+ // Create first snapshot and tag it
+ table.newAppend().appendFile(FILE_A).commit();
+ long taggedSnapshotId = table.currentSnapshot().snapshotId();
+ table
+ .manageSnapshots()
+ .createTag("production", taggedSnapshotId)
+ .setMaxRefAgeMs("production", Long.MAX_VALUE)
+ .commit();
+
+ // Add more data
+ table.newAppend().appendFile(FILE_B).commit();
+
+ // Serialize and deserialize
+ SerializableTable serializableTable = (SerializableTable)
SerializableTable.copyOf(table);
+
+ // Test with Java serialization
+ Table javaSerialized = TestHelpers.roundTripSerialize(serializableTable);
+ verifySnapshotRefs(javaSerialized, taggedSnapshotId);
+
+ // Test with Kryo serialization
+ Table kryoSerialized =
TestHelpers.KryoHelpers.roundTripSerialize(serializableTable);
+ verifySnapshotRefs(kryoSerialized, taggedSnapshotId);
+ }
+
+ private void verifySnapshotRefs(Table deserialized, long taggedSnapshotId) {
+ // Verify refs are preserved
+ assertThat(deserialized.refs()).isNotNull();
+ assertThat(deserialized.refs()).containsKey("production");
+
assertThat(deserialized.refs().get("production").snapshotId()).isEqualTo(taggedSnapshotId);
Review Comment:
these 3 statements can be simplified to a single assertion:
`assertThat(deserialized.refs()).containsEntry("production", ...)`
--
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]