diff --git a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java index ee61350ab1d32db96a234f6444836f76e23f3251..631a6f4d5eaf6e0d0e9b8eb5f1a9e695b6e90087 100644 --- a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java +++ b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java @@ -66,15 +66,13 @@ public boolean accept(Path path) { /** * Variant of Path.makeQualified that qualifies the input path against the default file system * indicated by the configuration - * + *

* This does not require a FileSystem handle in most cases - only requires the Filesystem URI. * This saves the cost of opening the Filesystem - which can involve RPCs - as well as cause * errors * - * @param path - * path to be fully qualified - * @param conf - * Configuration file + * @param path path to be fully qualified + * @param conf Configuration file * @return path qualified relative to default file system */ public static Path makeQualified(Path path, Configuration conf) throws IOException { @@ -128,14 +126,14 @@ public static String makePartName(List partCols, List vals) { /** * Makes a valid partition name. - * @param partCols The partition keys' names - * @param vals The partition values - * @param defaultStr - * The default name given to a partition value if the respective value is empty or null. + * + * @param partCols The partition keys' names + * @param vals The partition values + * @param defaultStr The default name given to a partition value if the respective value is empty or null. * @return An escaped, valid partition name. */ public static String makePartName(List partCols, List vals, - String defaultStr) { + String defaultStr) { StringBuilder name = new StringBuilder(); for (int i = 0; i < partCols.size(); i++) { if (i > 0) { @@ -151,12 +149,13 @@ public static String makePartName(List partCols, List vals, /** * default directory will have the same depth as number of skewed columns * this will make future operation easy like DML merge, concatenate merge + * * @param skewedCols * @param name * @return */ public static String makeDefaultListBucketingDirName(List skewedCols, - String name) { + String name) { String lbDirName; String defaultDir = FileUtils.escapePathName(name); StringBuilder defaultDirPath = new StringBuilder(); @@ -172,8 +171,9 @@ public static String makeDefaultListBucketingDirName(List skewedCols, /** * Makes a valid list bucketing directory name. + * * @param lbCols The skewed keys' names - * @param vals The skewed values + * @param vals The skewed values * @return An escaped, valid list bucketing directory name. */ public static String makeListBucketingDirName(List lbCols, List vals) { @@ -205,6 +205,7 @@ public static String makeListBucketingDirName(List lbCols, List // the list below, then the drop partition fails to work. static BitSet charToEscape = new BitSet(128); + static { for (char c = 0; c < ' '; c++) { charToEscape.set(c); @@ -214,21 +215,21 @@ public static String makeListBucketingDirName(List lbCols, List * ASCII 01-1F are HTTP control characters that need to be escaped. * \u000A and \u000D are \n and \r, respectively. */ - char[] clist = new char[] {'\u0001', '\u0002', '\u0003', '\u0004', - '\u0005', '\u0006', '\u0007', '\u0008', '\u0009', '\n', '\u000B', - '\u000C', '\r', '\u000E', '\u000F', '\u0010', '\u0011', '\u0012', - '\u0013', '\u0014', '\u0015', '\u0016', '\u0017', '\u0018', '\u0019', - '\u001A', '\u001B', '\u001C', '\u001D', '\u001E', '\u001F', - '"', '#', '%', '\'', '*', '/', ':', '=', '?', '\\', '\u007F', '{', - '[', ']', '^'}; + char[] clist = new char[]{'\u0001', '\u0002', '\u0003', '\u0004', + '\u0005', '\u0006', '\u0007', '\u0008', '\u0009', '\n', '\u000B', + '\u000C', '\r', '\u000E', '\u000F', '\u0010', '\u0011', '\u0012', + '\u0013', '\u0014', '\u0015', '\u0016', '\u0017', '\u0018', '\u0019', + '\u001A', '\u001B', '\u001C', '\u001D', '\u001E', '\u001F', + '"', '#', '%', '\'', '*', '/', ':', '=', '?', '\\', '\u007F', '{', + '[', ']', '^'}; for (char c : clist) { charToEscape.set(c); } - if(Shell.WINDOWS){ + if (Shell.WINDOWS) { //On windows, following chars need to be escaped as well - char [] winClist = {' ', '<','>','|'}; + char[] winClist = {' ', '<', '>', '|'}; for (char c : winClist) { charToEscape.set(c); } @@ -246,9 +247,9 @@ public static String escapePathName(String path) { /** * Escapes a path name. - * @param path The path to escape. - * @param defaultPath - * The default name for the path, if the given path is empty or null. + * + * @param path The path to escape. + * @param defaultPath The default name for the path, if the given path is empty or null. * @return An escaped path name. */ public static String escapePathName(String path, String defaultPath) { @@ -304,17 +305,12 @@ public static String unescapePathName(String path) { * Recursively lists status for all files starting from a particular directory (or individual file * as base case). * - * @param fs - * file system - * - * @param fileStatus - * starting point in file system - * - * @param results - * receives enumeration of all files found + * @param fs file system + * @param fileStatus starting point in file system + * @param results receives enumeration of all files found */ public static void listStatusRecursively(FileSystem fs, FileStatus fileStatus, - List results) throws IOException { + List results) throws IOException { if (fileStatus.isDir()) { for (FileStatus stat : fs.listStatus(fileStatus.getPath(), new PathFilter() { @@ -463,46 +459,48 @@ public static boolean isOwnerOfFileHierarchy(FileSystem fs, FileStatus fileStatu /** * Creates the directory and all necessary parent directories. - * @param fs FileSystem to use - * @param f path to create. + * + * @param fs FileSystem to use + * @param pathOfNewDir path to create. * @param inheritPerms whether directory inherits the permission of the last-existing parent path - * @param conf Hive configuration + * @param conf Hive configuration * @return true if directory created successfully. False otherwise, including if it exists. * @throws IOException exception in creating the directory */ - public static boolean mkdir(FileSystem fs, Path f, boolean inheritPerms, Configuration conf) throws IOException { - LOG.info("Creating directory if it doesn't exist: " + f); + public static boolean mkdir(FileSystem fs, Path pathOfNewDir, boolean inheritPerms, Configuration conf) throws IOException { + LOG.info("Creating directory if it doesn't exist: " + pathOfNewDir); if (!inheritPerms) { //just create the directory - return fs.mkdirs(f); + return fs.mkdirs(pathOfNewDir); } else { //Check if the directory already exists. We want to change the permission //to that of the parent directory only for newly created directories. try { - return fs.getFileStatus(f).isDir(); + return fs.getFileStatus(pathOfNewDir).isDirectory(); } catch (FileNotFoundException ignore) { } - //inherit perms: need to find last existing parent path, and apply its permission on entire subtree. - Path path = f; + //inherit perms: need to find last existing parent pathOfExistingParent, and apply its permission on entire subtree. + Path pathOfExistingParent = pathOfNewDir; List pathsToSet = new ArrayList(); - while (!fs.exists(path)) { - pathsToSet.add(path); - path = path.getParent(); + while (!fs.exists(pathOfExistingParent)) { + pathsToSet.add(pathOfExistingParent); + pathOfExistingParent = pathOfExistingParent.getParent(); } - //at the end of this loop, path is the last-existing parent path. - boolean success = fs.mkdirs(f); + //at the end of this loop, pathOfExistingParent is the last-existing parent pathOfExistingParent. + FsPermission parentPerm = fs.getFileStatus(pathOfExistingParent).getPermission(); + + boolean success = fs.mkdirs(pathOfNewDir, parentPerm); if (!success) { return false; } else { - FsPermission parentPerm = fs.getFileStatus(path).getPermission(); String permString = Integer.toString(parentPerm.toShort(), 8); + FsShell fsShell = new FsShell(); + fsShell.setConf(conf); for (Path pathToSet : pathsToSet) { - LOG.info("Setting permission of parent directory: " + path.toString() + + LOG.info("Setting permission " + permString + " of parent directory: " + pathOfExistingParent.toString() + " on new directory: " + pathToSet.toString()); try { - FsShell fshell = new FsShell(); - fshell.setConf(conf); - fshell.run(new String[]{"-chmod", "-R", permString, pathToSet.toString()}); + fsShell.run(new String[]{"-chmod", "-R", permString, pathToSet.toString()}); } catch (Exception e) { LOG.warn("Error setting permissions of " + pathToSet, e); } @@ -516,10 +514,10 @@ public static boolean mkdir(FileSystem fs, Path f, boolean inheritPerms, Configu * Copies files between filesystems. */ public static boolean copy(FileSystem srcFS, Path src, - FileSystem dstFS, Path dst, - boolean deleteSource, - boolean overwrite, - HiveConf conf) throws IOException { + FileSystem dstFS, Path dst, + boolean deleteSource, + boolean overwrite, + HiveConf conf) throws IOException { boolean copied = FileUtil.copy(srcFS, src, dstFS, dst, deleteSource, overwrite, conf); boolean inheritPerms = conf.getBoolVar(HiveConf.ConfVars.HIVE_WAREHOUSE_SUBDIR_INHERIT_PERMS); if (copied && inheritPerms) { @@ -588,4 +586,30 @@ public static boolean moveToTrash(FileSystem fs, Path f, Configuration conf) thr return result; } + public static void rename(FileSystem fs, Path sourcePath, Path destPath, boolean inheritPerms, Configuration conf) throws IOException { + LOG.info("Copying " + sourcePath + " to " + destPath); + if (!inheritPerms) { + //just rename the directory + fs.rename(sourcePath, destPath); + } else { + //inherit perms: need to find parent pathOfExistingParent, and apply its permission on entire subtree. + Path pathOfExistingParent = destPath.getParent(); + FsPermission parentPerm = fs.getFileStatus(pathOfExistingParent).getPermission(); + + //rename the directory + fs.rename(sourcePath, destPath); + + String permString = Integer.toString(parentPerm.toShort(), 8); + FsShell fsShell = new FsShell(); + fsShell.setConf(conf); + LOG.info("Setting permission " + permString + " of parent directory: " + pathOfExistingParent.toString() + + " on new directory: " + destPath.toString() + " recursively"); + try { + fsShell.run(new String[]{"-chmod", "-R", permString, destPath.toString()}); + } catch (Exception e) { + LOG.warn("Error setting permissions of " + destPath, e); + } + } + } + } diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java index 4f566d2a2999eecea7018836505208ae80662e97..8c2e21c6a3d72137bed68e3084ce59e273aa7d20 100644 --- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java +++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java @@ -147,6 +147,35 @@ public void testStaticPartitionPerms() throws Exception { } @Test + public void testAlterSinglePartitionPerms() throws Exception { + String tableName = "alterpart"; + CommandProcessorResponse ret = driver.run("CREATE TABLE " + tableName + " (key string, value string) partitioned by (part int)"); + Assert.assertEquals(0,ret.getResponseCode()); + + assertExistence(testDir + "/" + tableName); + setPermissions(testDir + "/" + tableName, FsPermission.createImmutable((short) 0777)); + + ret = driver.run("insert into table " + tableName + " partition(part='1') select key,value from mysrc"); + Assert.assertEquals(0,ret.getResponseCode()); + + Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part=1").toString()); + + //change permission of table + setPermissions(testDir + "/" + tableName, FsPermission.createImmutable((short) 0755)); + + //alter partition + ret = driver.run("alter table " + tableName + " partition (part='1') rename to partition (part='2')"); + Assert.assertEquals(0,ret.getResponseCode()); + + Assert.assertEquals("rwxr-xr-x", getPermissions(testDir + "/" + tableName + "/part=2").toString()); + + Assert.assertTrue(listChildrenPerms(testDir + "/" + tableName + "/part=2").size() > 0); + for (FsPermission perm : listChildrenPerms(testDir + "/" + tableName + "/part=2")) { + Assert.assertEquals("rwxr-xr-x", perm.toString()); + } + } + + @Test public void testAlterPartitionPerms() throws Exception { String tableName = "alterpart"; CommandProcessorResponse ret = driver.run("CREATE TABLE " + tableName + " (key string, value string) partitioned by (part1 int, part2 int, part3 int)"); @@ -158,17 +187,24 @@ public void testAlterPartitionPerms() throws Exception { ret = driver.run("insert into table " + tableName + " partition(part1='1',part2='1',part3='1') select key,value from mysrc"); Assert.assertEquals(0,ret.getResponseCode()); + Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part1=1").toString()); + Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part1=1/part2=1").toString()); + Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part1=1/part2=1/part3=1").toString()); + + //change permission of table + setPermissions(testDir + "/" + tableName, FsPermission.createImmutable((short) 0755)); + //alter partition ret = driver.run("alter table " + tableName + " partition (part1='1',part2='1',part3='1') rename to partition (part1='2',part2='2',part3='2')"); Assert.assertEquals(0,ret.getResponseCode()); - Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part1=2").toString()); - Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part1=2/part2=2").toString()); - Assert.assertEquals("rwxrwxrwx", getPermissions(testDir + "/" + tableName + "/part1=2/part2=2/part3=2").toString()); + Assert.assertEquals("rwxr-xr-x", getPermissions(testDir + "/" + tableName + "/part1=2").toString()); + Assert.assertEquals("rwxr-xr-x", getPermissions(testDir + "/" + tableName + "/part1=2/part2=2").toString()); + Assert.assertEquals("rwxr-xr-x", getPermissions(testDir + "/" + tableName + "/part1=2/part2=2/part3=2").toString()); Assert.assertTrue(listChildrenPerms(testDir + "/" + tableName + "/part1=2/part2=2/part3=2").size() > 0); for (FsPermission perm : listChildrenPerms(testDir + "/" + tableName + "/part1=2/part2=2/part3=2")) { - Assert.assertEquals("rwxrwxrwx", perm.toString()); + Assert.assertEquals("rwxr-xr-x", perm.toString()); } } diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java index 221b01043b3520211643847c00b17b328ad2b9c0..3126880aa1a1c5ed93c9576e84c980511e86145b 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java @@ -398,7 +398,7 @@ public Partition alterPartition(final RawStore msdb, Warehouse wh, final String if (!wh.mkdirs(destParentPath, true)) { throw new IOException("Unable to create path " + destParentPath); } - srcFs.rename(srcPath, destPath); + wh.renameDir(srcPath, destPath, true); LOG.info("rename done!"); } } catch (IOException e) { diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java b/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java index c1790b427e7fa64780fdf0a580f28135d6a4ff8d..d96ede5e002465d124db2c756eb2524e17def264 100755 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java @@ -198,10 +198,14 @@ public boolean mkdirs(Path f, boolean inheritPermCandidate) throws MetaException } public boolean renameDir(Path sourcePath, Path destPath) throws MetaException { + return renameDir(sourcePath, destPath, false); + } + + public boolean renameDir(Path sourcePath, Path destPath, boolean inheritPerms) throws MetaException { FileSystem fs = null; try { fs = getFs(sourcePath); - fs.rename(sourcePath, destPath); + FileUtils.rename(fs, sourcePath, destPath, inheritPerms, conf); return true; } catch (Exception ex) { MetaStoreUtils.logAndThrowMetaException(ex); @@ -556,5 +560,4 @@ public static String makePartName(List partCols, values.addAll(partSpec.values()); return values; } - }