mchades commented on code in PR #4262:
URL: https://github.com/apache/gravitino/pull/4262#discussion_r1701077714
##########
catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveIT.java:
##########
@@ -1637,6 +1637,38 @@ void testCustomCatalogOperations() {
catalogName + "_not_exists",
"org.apache.gravitino.catalog.not.exists"));
}
+ @Test
+ void testAlterCatalogProperties() {
+ Map<String, String> properties = Maps.newHashMap();
+ String nameOfCatalog = GravitinoITUtils.genRandomName("catalog");
+ // Wrong Hive HIVE_METASTORE_URIS
+ String wrongHiveMetastoreURI = HIVE_METASTORE_URIS + "_wrong";
+ properties.put(METASTORE_URIS, wrongHiveMetastoreURI);
+ Catalog createdCatalog =
+ metalake.createCatalog(
+ nameOfCatalog, Catalog.Type.RELATIONAL, provider, "comment",
properties);
+ Assertions.assertEquals(wrongHiveMetastoreURI,
createdCatalog.properties().get(METASTORE_URIS));
+
+ // As it's wrong metastore uri, it should throw exception.
+ Assertions.assertThrows(
+ Exception.class,
+ () -> createdCatalog.asSchemas().createSchema("schema", "comment",
ImmutableMap.of()));
+
+ Catalog newCatalog =
+ metalake.alterCatalog(
+ nameOfCatalog, CatalogChange.setProperty(METASTORE_URIS,
HIVE_METASTORE_URIS));
+ Assertions.assertEquals(HIVE_METASTORE_URIS,
newCatalog.properties().get(METASTORE_URIS));
+
+ // The URI has restored, so it should not throw exception.
Review Comment:
should we test creating the same schema after altering catalog?
##########
catalogs/catalog-hadoop/src/test/java/org/apache/gravitino/catalog/hadoop/integration/test/HadoopCatalogIT.java:
##########
@@ -137,6 +138,29 @@ private static void dropSchema() {
Assertions.assertFalse(catalog.asSchemas().schemaExists(schemaName));
}
+ @Test
+ void testAlterCatalogLocation() {
+ String catalogName =
GravitinoITUtils.genRandomName("test_alter_catalog_location");
+ String location = defaultBaseLocation();
+ String newLocation = location + "/new_location";
+
+ Map<String, String> catalogProperties = ImmutableMap.of("location",
location);
+ // Create a catalog using location
+ Catalog filesetCatalog =
+ metalake.createCatalog(
+ catalogName, Catalog.Type.FILESET, provider, "comment",
catalogProperties);
+
+ Assertions.assertEquals(location,
filesetCatalog.properties().get("location"));
+
+ // Now try to alter the location and change it to `newLocation`.
+ Catalog modifiedCatalog =
+ metalake.alterCatalog(catalogName,
CatalogChange.setProperty("location", newLocation));
+
+ Assertions.assertEquals(newLocation,
modifiedCatalog.properties().get("location"));
Review Comment:
Considering the inheritability of fileset location, I believe that testing
only a catalog without schema is not enough.
##########
catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHiveIT.java:
##########
@@ -1637,6 +1637,38 @@ void testCustomCatalogOperations() {
catalogName + "_not_exists",
"org.apache.gravitino.catalog.not.exists"));
}
+ @Test
+ void testAlterCatalogProperties() {
+ Map<String, String> properties = Maps.newHashMap();
+ String nameOfCatalog = GravitinoITUtils.genRandomName("catalog");
+ // Wrong Hive HIVE_METASTORE_URIS
+ String wrongHiveMetastoreURI = HIVE_METASTORE_URIS + "_wrong";
+ properties.put(METASTORE_URIS, wrongHiveMetastoreURI);
+ Catalog createdCatalog =
+ metalake.createCatalog(
+ nameOfCatalog, Catalog.Type.RELATIONAL, provider, "comment",
properties);
+ Assertions.assertEquals(wrongHiveMetastoreURI,
createdCatalog.properties().get(METASTORE_URIS));
+
+ // As it's wrong metastore uri, it should throw exception.
+ Assertions.assertThrows(
+ Exception.class,
+ () -> createdCatalog.asSchemas().createSchema("schema", "comment",
ImmutableMap.of()));
Review Comment:
Can you clear the exception type?
--
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]
