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;
}
-
}