[GitHub] [incubator-iceberg] aokolnychyi commented on a change in pull request #253: [WIP] Register existing tables in Iceberg HiveCatalog

[GitBox <gi...@apache.org>]

aokolnychyi commented on a change in pull request #253: [WIP] Register existing tables in Iceberg HiveCatalog
URL: https://github.com/apache/incubator-iceberg/pull/253#discussion_r300453831
 
 

 ##########
 File path: hive/src/test/java/org/apache/iceberg/hive/HiveTableTest.java
 ##########
 @@ -223,4 +223,80 @@ public void testConcurrentFastAppends() {
     icebergTable.refresh();
     Assert.assertEquals(20, icebergTable.currentSnapshot().manifests().size());
   }
+
+  @Test
+  public void testRegisterTable() throws TException {
+    org.apache.hadoop.hive.metastore.api.Table originalTable = metastoreClient.getTable(DB_NAME, TABLE_NAME);
+
+    Map<String, String> originalParams = originalTable.getParameters();
+    Assert.assertNotNull(originalParams);
+    Assert.assertTrue(ICEBERG_TABLE_TYPE_VALUE.equalsIgnoreCase(originalParams.get(TABLE_TYPE_PROP)));
+    Assert.assertTrue("EXTERNAL_TABLE".equalsIgnoreCase(originalTable.getTableType()));
+
+    catalog.dropTable(TABLE_IDENTIFIER);
+    Assert.assertFalse(catalog.tableExists(TABLE_IDENTIFIER));
+
+    List<String> metadataVersionFiles = metadataVersionFiles(TABLE_NAME);
+    Assert.assertEquals(1, metadataVersionFiles.size());
+
+    catalog.registerTable(TABLE_IDENTIFIER, "file:" + metadataVersionFiles.get(0));
+
+    org.apache.hadoop.hive.metastore.api.Table newTable = metastoreClient.getTable(DB_NAME, TABLE_NAME);
+
+    Assert.assertEquals("Storage descriptor shouldn't change", originalTable.getSd(), newTable.getSd());
+    Assert.assertEquals("Params shouldn't change", originalParams, newTable.getParameters());
+  }
+
+  @Test
+  public void testCloneTable() throws IOException {
+    Table table = catalog.loadTable(TABLE_IDENTIFIER);
+
+    List<String> metadataVersionFiles = metadataVersionFiles(TABLE_NAME);
+    Assert.assertEquals(1, metadataVersionFiles.size());
+
+    TableIdentifier anotherTableIdentifier = TableIdentifier.of(DB_NAME, TABLE_NAME + "_new");
+    Table anotherTable = catalog.registerTable(anotherTableIdentifier, metadataVersionFiles.get(0));
+
+    GenericRecordBuilder recordBuilder = new GenericRecordBuilder(AvroSchemaUtil.convert(schema, "test"));
+    List<GenericData.Record> records = Lists.newArrayList(
+        recordBuilder.set("id", 1L).build(),
+        recordBuilder.set("id", 2L).build(),
+        recordBuilder.set("id", 3L).build()
+    );
+
+    String fileLocation = table.location().replace("file:", "") + "/data/file-1.avro";
+    try (FileAppender<GenericData.Record> writer = Avro.write(Files.localOutput(fileLocation))
+        .schema(schema)
+        .named("test")
+        .build()) {
+      for (GenericData.Record rec : records) {
+        writer.add(rec);
+      }
+    }
+    DataFile file = DataFiles.builder(table.spec())
+        .withRecordCount(3)
+        .withPath(fileLocation)
+        .withFileSizeInBytes(Files.localInput(fileLocation).getLength())
+        .build();
+    table.newAppend().appendFile(file).commit();
+
+    String anotherFileLocation = anotherTable.location().replace("file:", "") + "/data/file-2.avro";
+    try (FileAppender<GenericData.Record> writer = Avro.write(Files.localOutput(anotherFileLocation))
+        .schema(schema)
+        .named("test")
+        .build()) {
+      for (GenericData.Record rec : records) {
+        writer.add(rec);
+      }
+    }
+    DataFile anotherFile = DataFiles.builder(anotherTable.spec())
+        .withRecordCount(3)
+        .withPath(anotherFileLocation)
+        .withFileSizeInBytes(Files.localInput(anotherFileLocation).getLength())
+        .build();
+    anotherTable.newAppend().appendFile(anotherFile).commit();
+
+    // verify that both tables continue to function independently
+    Assert.assertNotEquals(table.currentSnapshot().manifests(), anotherTable.currentSnapshot().manifests());
 
 Review comment:
   I have to say that the registered table won't have `previous_metadata_location` in its table properties. This doesn't seem like an issue to me.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org