commit 89c8572daab302b36d18960a075530a4de214fa5 Author: Andrew Sherman Date: Mon Feb 5 14:41:30 2018 -0800 HIVE-18625: make SessionState.createPath() check the result of directory creation. diff --git ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java index 3946d4a3445fc4df7475a7ba165d3980ce43f849..d88f445b37fc452c479e09facb91cae927d1b707 100644 --- ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java +++ ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java @@ -760,7 +760,8 @@ private Path createRootHDFSDir(HiveConf conf) throws IOException { * @return * @throws IOException */ - private static void createPath(HiveConf conf, Path path, String permission, boolean isLocal, + @VisibleForTesting + static void createPath(HiveConf conf, Path path, String permission, boolean isLocal, boolean isCleanUp) throws IOException { FsPermission fsPermission = new FsPermission(permission); FileSystem fs; @@ -770,7 +771,10 @@ private static void createPath(HiveConf conf, Path path, String permission, bool fs = path.getFileSystem(conf); } if (!fs.exists(path)) { - fs.mkdirs(path, fsPermission); + boolean created = fs.mkdirs(path, fsPermission); + if (!created) { + throw new IOException("mkdirs failed to create " + path + " on fs " + fs); + } String dirType = isLocal ? "local" : "HDFS"; LOG.info("Created " + dirType + " directory: " + path.toString()); } diff --git ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java index 8750196620dc03365b6c9eff1668762c463bc312..0b57fb50471c20b4ad50a8e9c29cb10197618b52 100644 --- ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java +++ ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java @@ -19,6 +19,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.File; import java.io.IOException; @@ -27,6 +29,10 @@ import java.util.Collection; import org.apache.commons.io.FileUtils; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.LocalFileSystem; +import org.apache.hadoop.fs.ParentNotDirectoryException; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.metastore.Warehouse; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -257,4 +263,51 @@ public void testReloadExistingAuxJars2() { } } } + + /** + * Unit test for SessionState.createPath() + */ + @Test + public void testCreatePath() throws Exception { + HiveConf conf = new HiveConf(); + LocalFileSystem localFileSystem = FileSystem.getLocal(conf); + + Path repeatedCreate = new Path("repeatedCreate"); + SessionState.createPath(conf, repeatedCreate, "700", true, true); + assertTrue(localFileSystem.exists(repeatedCreate)); + // second time will complete silently + SessionState.createPath(conf, repeatedCreate, "700", true, true); + + Path fileNotDirectory = new Path("fileNotDirectory"); + localFileSystem.create(fileNotDirectory); + localFileSystem.deleteOnExit(fileNotDirectory); + + // Show we cannot create a child of a file + try { + SessionState.createPath(conf, new Path(fileNotDirectory, "child"), "700", true, true); + fail("did not get expected exception"); + } catch (ParentNotDirectoryException e) { + assertTrue(e.getMessage().contains("Parent path is not a directory")); + } + + // Show we cannot create a child of a null directory + try { + //noinspection ConstantConditions + SessionState.createPath(conf, new Path((String) null, "hoo"), "700", true, true); + fail("did not get expected exception"); + } catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Can not create a Path from a null string")); + } + + // Create a directory with no permissions + Path noPermissions = new Path("noPermissions"); + SessionState.createPath(conf, noPermissions, "000", true, true); + // Show we cannot create a child of the directory with no permissions + try { + SessionState.createPath(conf, new Path(noPermissions, "child"), "700", true, true); + fail("did not get expected exception"); + } catch (IOException e) { + assertTrue(e.getMessage().contains("mkdirs failed to create noPermissions/child")); + } + } }