marton-bod commented on a change in pull request #2123:
URL: https://github.com/apache/iceberg/pull/2123#discussion_r561070861



##########
File path: 
mr/src/test/java/org/apache/iceberg/mr/hive/TestHiveIcebergStorageHandlerNoScan.java
##########
@@ -521,4 +438,92 @@ public void 
testCreateTableWithNotSupportedTypesWithAutoConversion() {
       shell.executeStatement("DROP TABLE not_supported_types");
     }
   }
+
+  @Test
+  public void testIcebergAndHmsTableProperties() throws TException, 
InterruptedException {
+    TableIdentifier identifier = TableIdentifier.of("default", "customers");
+
+    shell.executeStatement(String.format("CREATE EXTERNAL TABLE 
default.customers " +
+        "STORED BY 'org.apache.iceberg.mr.hive.HiveIcebergStorageHandler' %s" +
+        "TBLPROPERTIES ('%s'='%s', '%s'='%s', '%s'='%s')",
+        testTables.locationForCreateTableSQL(identifier), // we need the 
location for HadoopTable based tests only
+        InputFormatConfig.TABLE_SCHEMA, 
SchemaParser.toJson(HiveIcebergStorageHandlerTestUtils.CUSTOMER_SCHEMA),
+        InputFormatConfig.PARTITION_SPEC, PartitionSpecParser.toJson(SPEC),
+        "custom_property", "initial_val"));
+
+
+    // Check the Iceberg table parameters
+    org.apache.iceberg.Table icebergTable = testTables.loadTable(identifier);
+
+    Map<String, String> expectedIcebergProperties = new HashMap<>();
+    expectedIcebergProperties.put("custom_property", "initial_val");
+    expectedIcebergProperties.put("EXTERNAL", "TRUE");
+    expectedIcebergProperties.put("storage_handler", 
HiveIcebergStorageHandler.class.getName());
+    if (Catalogs.hiveCatalog(shell.getHiveConf())) {
+      expectedIcebergProperties.put(TableProperties.ENGINE_HIVE_ENABLED, 
"true");
+    }
+    if (MetastoreUtil.hive3PresentOnClasspath()) {
+      expectedIcebergProperties.put("bucketing_version", "2");
+    }
+    Assert.assertEquals(expectedIcebergProperties, icebergTable.properties());
+
+    // Check the HMS table parameters
+    org.apache.hadoop.hive.metastore.api.Table hmsTable = 
getHmsTable("default", "customers");
+    Map<String, String> hmsParams = hmsTable.getParameters()
+        .entrySet().stream()
+        .filter(e -> !IGNORED_PARAMS.contains(e.getKey()))
+        .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+    if (Catalogs.hiveCatalog(shell.getHiveConf())) {
+      Assert.assertEquals(9, hmsParams.size());
+      Assert.assertEquals("initial_val", hmsParams.get("custom_property"));
+      Assert.assertEquals("TRUE", 
hmsParams.get(InputFormatConfig.EXTERNAL_TABLE_PURGE));
+      Assert.assertEquals("TRUE", hmsParams.get("EXTERNAL"));
+      Assert.assertEquals("true", 
hmsParams.get(TableProperties.ENGINE_HIVE_ENABLED));
+      Assert.assertEquals(HiveIcebergStorageHandler.class.getName(),
+          hmsParams.get(hive_metastoreConstants.META_TABLE_STORAGE));
+      
Assert.assertEquals(BaseMetastoreTableOperations.ICEBERG_TABLE_TYPE_VALUE.toUpperCase(),
+          hmsParams.get(BaseMetastoreTableOperations.TABLE_TYPE_PROP));
+      
Assert.assertTrue(hmsParams.get(BaseMetastoreTableOperations.METADATA_LOCATION_PROP)

Review comment:
       Yes, good idea. Added some metadata location checks




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