veghlaci05 commented on code in PR #4297: URL: https://github.com/apache/hive/pull/4297#discussion_r1188287825
########## itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestWarehouseExternalDir.java: ########## @@ -44,114 +44,94 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; - +import org.apache.hadoop.hive.metastore.api.Database; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hadoop.hive.metastore.Warehouse; - [email protected]("HIVE-25266") public class TestWarehouseExternalDir { - private static final Logger LOG = LoggerFactory.getLogger(TestWarehouseExternalDir.class); + private static final Logger LOG = LoggerFactory.getLogger(TestWarehouseExternalDir.class); + + private static MiniHS2 miniHS2; + private static Hive db; - static MiniHS2 miniHS2; - static Hive db; - static Connection conn; + private static HiveConf conf; + private static Connection conn; Review Comment: I would remove this field. 1. `beforeTest()` creates a connection (line 88) 2. This is **_not closed_** but reassigned with a new connection in setUp() (line 119) 3. `tearDown()` closes the connection after every test (but there's only one test) Even despite the lot of maintenance code this results in an unclosed connection. I would simply create and properly close local connection variables both in `beforeTest()` and `testExternalDefaultPaths()` ########## itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestWarehouseExternalDir.java: ########## @@ -174,19 +155,19 @@ public void testManagedPaths() throws Exception { @Test public void testExternalDefaultPaths() throws Exception { try (Statement stmt = conn.createStatement()) { - stmt.execute("create external table default.twed_ext1(c1 string)"); - Table tab = db.getTable("default", "twed_ext1"); + stmt.execute("create external table IF NOT EXISTS default.twed_ext1(c1 string)"); Review Comment: Why the `if not exists` clause required? -- 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]
