Repository: hive Updated Branches: refs/heads/master 03a1e6247 -> 111ed0964
HIVE-18625: SessionState Not Checking For Directory Creation Result (Andrew Sherman, reviewed by Sahil Takiar) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/111ed096 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/111ed096 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/111ed096 Branch: refs/heads/master Commit: 111ed096496c37674601173cfeaa40cbf948f878 Parents: 03a1e62 Author: Andrew Sherman <asher...@cloudera.com> Authored: Tue Feb 20 11:28:20 2018 -0800 Committer: Sahil Takiar <stak...@cloudera.com> Committed: Tue Feb 20 11:28:34 2018 -0800 ---------------------------------------------------------------------- .../hadoop/hive/ql/session/SessionState.java | 7 ++- .../hive/ql/session/TestSessionState.java | 53 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/111ed096/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java ---------------------------------------------------------------------- diff --git a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java index dfc2dfa..0071a9a 100644 --- a/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java +++ b/ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java @@ -740,7 +740,8 @@ public class SessionState { * @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; @@ -750,7 +751,9 @@ public class SessionState { fs = path.getFileSystem(conf); } if (!fs.exists(path)) { - fs.mkdirs(path, fsPermission); + if (!fs.mkdirs(path, fsPermission)) { + throw new IOException("Failed to create directory " + path + " on fs " + fs.getUri()); + } String dirType = isLocal ? "local" : "HDFS"; LOG.info("Created " + dirType + " directory: " + path.toString()); } http://git-wip-us.apache.org/repos/asf/hive/blob/111ed096/ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java ---------------------------------------------------------------------- diff --git a/ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java b/ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java index 8750196..0fa1c81 100644 --- a/ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java +++ b/ql/src/test/org/apache/hadoop/hive/ql/session/TestSessionState.java @@ -19,6 +19,8 @@ package org.apache.hadoop.hive.ql.session; 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.Arrays; 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 class TestSessionState { } } } + + /** + * 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 creating a child of a file"); + } 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, "child"), "700", true, true); + fail("did not get expected exception creating a Path from a null string"); + } 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 creating a child of a directory with no permissions"); + } catch (IOException e) { + assertTrue(e.getMessage().contains("Failed to create directory noPermissions/child")); + } + } }