dengzhhu653 commented on code in PR #3477:
URL: https://github.com/apache/hive/pull/3477#discussion_r934387518
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue()
throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) +
"/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
Review Comment:
The failed tests seem have relation with the changes, as the `db` is null.
you should create `Database` first.
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1399,6 +1403,43 @@ public void create_database(final Database db)
}
}
+ @Override
+ public void create_database_req(final CreateDatabaseRequest req)
+ throws AlreadyExistsException, InvalidObjectException, MetaException
{
+ Database db = req.getDatabase();
+ boolean skipFSWrites = req.isSkipFSWrites();
+ startFunction("create_database_req", ": " + db.toString());
+ boolean success = false;
+ Exception ex = null;
+ if (!db.isSetCatalogName()) {
+ db.setCatalogName(getDefaultCatalog(conf));
+ }
+ try {
+ try {
+ if (null != get_database_core(db.getCatalogName(), db.getName())) {
+ throw new AlreadyExistsException("Database " + db.getName() + "
already exists");
+ }
+ } catch (NoSuchObjectException e) {
+ // expected
+ }
+ create_database_core(getMS(), db, skipFSWrites);
+ success = true;
+ } catch (Exception e) {
+ ex = e;
Review Comment:
the catch clause can be rewritten like:
` ex = e;
throw handleException(e)
.throwIfInstance(MetaException.class,
InvalidObjectException.class, AlreadyExistsException.class)
.defaultMetaException();`
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue()
throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
Review Comment:
seems we do not need introduce another method to perform tests
##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java:
##########
@@ -3717,4 +3683,42 @@ public void testDropDataConnectorIfNotExistsTrue()
throws Exception {
// No such data connector, ignore NoSuchObjectException
client.dropDataConnector("no_such_data_connector", true, false);
}
+
+ @Test
+
+ public void testIfFSWritesIsSkipped() throws Throwable {
+ skipFSWritesTester();
+ }
+ private void skipFSWritesTester() throws Throwable {
+ // create a database, check if the directory is created or not
+ // with true, the directory is not created
+ // with false, the directory is created
+ try {
+ // clear up any existing databases
+ silentDropDatabase(TEST_DB1_NAME);
+
+ String dbLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE_EXTERNAL) +
"/testdb1.db";
+ String mgdLocation =
+ MetastoreConf.getVar(conf, ConfVars.WAREHOUSE) + "/testdb1.db";
+
+ CreateDatabaseRequest req = new CreateDatabaseRequest();
+ req.setSkipFSWrites(true);
+ Database db = req.getDatabase();
+ db.setName(TEST_DB1_NAME);
+ client.createDatabase(req);
+
+ Path dbPath = new Path(db.getLocationUri());
+ FileSystem fs = FileSystem.get(new Path(dbLocation).toUri(), conf);
+ assertFalse("Database's file system directory is skipped", fs.exists(new
Path(dbLocation)));
+ fs = FileSystem.get(new Path(mgdLocation).toUri(), conf);
Review Comment:
maybe we should call `createDatabase` again with Database location setting
to `mgdLocation`.
--
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]
