You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by se...@apache.org on 2015/10/02 04:37:55 UTC
[20/22] hive git commit: HIVE-11995 : Remove repetitively setting
permissions in insert/load overwrite partition (Chaoyu Tang via Szehon)
HIVE-11995 : Remove repetitively setting permissions in insert/load overwrite partition (Chaoyu Tang via Szehon)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/50744231
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/50744231
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/50744231
Branch: refs/heads/llap
Commit: 507442319985198466b4f6c2ba18c6b068d8435e
Parents: 467a117
Author: Szehon Ho <sz...@cloudera.com>
Authored: Thu Oct 1 15:29:36 2015 -0700
Committer: Szehon Ho <sz...@cloudera.com>
Committed: Thu Oct 1 15:29:36 2015 -0700
----------------------------------------------------------------------
.../hive/ql/security/FolderPermissionBase.java | 53 +++++++--
.../apache/hadoop/hive/ql/metadata/Hive.java | 108 ++++---------------
2 files changed, 65 insertions(+), 96 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/50744231/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java
index d98082f..d7149a7 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java
@@ -261,6 +261,7 @@ public abstract class FolderPermissionBase {
//insert overwrite test
setPermission(warehouseDir + "/" + tableName, 1);
+ setPermission(warehouseDir + "/" + tableName + "/part1=1", 1);
ret = driver.run("insert overwrite table " + tableName + " partition(part1='1') select key,value from mysrc where part1='1' and part2='1'");
Assert.assertEquals(0, ret.getResponseCode());
@@ -297,6 +298,9 @@ public abstract class FolderPermissionBase {
//insert overwrite test
setPermission(warehouseDir + "/" + tableName, 1);
+ setPermission(warehouseDir + "/" + tableName + "/part1=1", 1);
+ setPermission(warehouseDir + "/" + tableName + "/part1=1/part2=1", 1);
+
ret = driver.run("insert overwrite table " + tableName + " partition(part1='1', part2='1') select key,value from mysrc where part1='1' and part2='1'");
Assert.assertEquals(0, ret.getResponseCode());
@@ -325,8 +329,9 @@ public abstract class FolderPermissionBase {
verifyDualPartitionTable(warehouseDir + "/" + tableName, 0);
- //Insert overwrite test, with permission set 1.
- setPermission(warehouseDir + "/" + tableName, 1);
+ //Insert overwrite test, with permission set 1. We need reset existing partitions to 1 since the permissions
+ //should be inherited from existing partition
+ setDualPartitionTable(warehouseDir + "/" + tableName, 1);
ret = driver.run("insert overwrite table " + tableName + " partition (part1,part2) select key,value,part1,part2 from mysrc");
Assert.assertEquals(0, ret.getResponseCode());
@@ -348,8 +353,9 @@ public abstract class FolderPermissionBase {
Assert.assertEquals(0,ret.getResponseCode());
verifySinglePartition(tableLoc, 0);
- //Insert overwrite test, with permission set 1.
- setPermission(tableLoc, 1);
+ //Insert overwrite test, with permission set 1. We need reset existing partitions to 1 since the permissions
+ //should be inherited from existing partition
+ setSinglePartition(tableLoc, 1);
ret = driver.run("insert overwrite table " + tableName + " partition (part1) select key,value,part1 from mysrc");
Assert.assertEquals(0,ret.getResponseCode());
verifySinglePartition(tableLoc, 1);
@@ -458,6 +464,9 @@ public abstract class FolderPermissionBase {
//case1B: load data local into overwrite non-partitioned-table
setPermission(warehouseDir + "/" + tableName, 1);
+ for (String child : listStatus(tableLoc)) {
+ setPermission(child, 1);
+ }
ret = driver.run("load data local inpath '" + dataFilePath + "' overwrite into table " + tableName);
Assert.assertEquals(0,ret.getResponseCode());
@@ -485,8 +494,13 @@ public abstract class FolderPermissionBase {
verifyPermission(child);
}
- //case 2B: insert data overwrite into non-partitioned table.
+ //case 2B: insert data overwrite into partitioned table. set testing table/partition folder hierarchy 1
+ //local load overwrite just overwrite the existing partition content but not the permission
setPermission(tableLoc, 1);
+ setPermission(partLoc, 1);
+ for (String child : listStatus(partLoc)) {
+ setPermission(child, 1);
+ }
ret = driver.run("LOAD DATA LOCAL INPATH '" + dataFilePath + "' OVERWRITE INTO TABLE " + tableName + " PARTITION (part1='1',part2='1')");
Assert.assertEquals(0,ret.getResponseCode());
@@ -521,6 +535,10 @@ public abstract class FolderPermissionBase {
//case1B: load data into overwrite non-partitioned-table
setPermission(warehouseDir + "/" + tableName, 1);
+ for (String child : listStatus(tableLoc)) {
+ setPermission(child, 1);
+ }
+
fs.copyFromLocalFile(dataFilePath, new Path(location));
ret = driver.run("load data inpath '" + location + "' overwrite into table " + tableName);
Assert.assertEquals(0,ret.getResponseCode());
@@ -550,8 +568,15 @@ public abstract class FolderPermissionBase {
verifyPermission(child);
}
- //case 2B: insert data overwrite into non-partitioned table.
+ //case 2B: insert data overwrite into partitioned table. set testing table/partition folder hierarchy 1
+ //load overwrite just overwrite the existing partition content but not the permission
setPermission(tableLoc, 1);
+ setPermission(partLoc, 1);
+ Assert.assertTrue(listStatus(partLoc).size() > 0);
+ for (String child : listStatus(partLoc)) {
+ setPermission(child, 1);
+ }
+
fs.copyFromLocalFile(dataFilePath, new Path(location));
ret = driver.run("LOAD DATA INPATH '" + location + "' OVERWRITE INTO TABLE " + tableName + " PARTITION (part1='1',part2='1')");
Assert.assertEquals(0,ret.getResponseCode());
@@ -693,7 +718,12 @@ public abstract class FolderPermissionBase {
assertExistence(partition);
verifyPermission(partition);
}
-
+
+ private void setSinglePartition(String tableLoc, int index) throws Exception {
+ setPermission(tableLoc + "/part1=1", index);
+ setPermission(tableLoc + "/part1=2", index);
+ }
+
private void verifySinglePartition(String tableLoc, int index) throws Exception {
verifyPermission(tableLoc + "/part1=1", index);
verifyPermission(tableLoc + "/part1=2", index);
@@ -709,6 +739,15 @@ public abstract class FolderPermissionBase {
}
}
+ private void setDualPartitionTable(String baseTablePath, int index) throws Exception {
+ setPermission(baseTablePath, index);
+ setPermission(baseTablePath + "/part1=1", index);
+ setPermission(baseTablePath + "/part1=1/part2=1", index);
+
+ setPermission(baseTablePath + "/part1=2", index);
+ setPermission(baseTablePath + "/part1=2/part2=2", index);
+ }
+
private void verifyDualPartitionTable(String baseTablePath, int index) throws Exception {
verifyPermission(baseTablePath, index);
verifyPermission(baseTablePath + "/part1=1", index);
http://git-wip-us.apache.org/repos/asf/hive/blob/50744231/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
----------------------------------------------------------------------
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
index 10cafb6..8efbb05 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
@@ -2932,8 +2932,7 @@ private void constructOneLBLocationMap(FileStatus fSta,
LOG.info("No sources specified to move: " + srcf);
return;
}
- List<List<Path[]>> result = checkPaths(conf, destFs, srcs, srcFs, destf,
- true);
+ List<List<Path[]>> result = checkPaths(conf, destFs, srcs, srcFs, destf, true);
if (oldPath != null) {
try {
@@ -2945,9 +2944,6 @@ private void constructOneLBLocationMap(FileStatus fSta,
if (FileUtils.isSubDir(oldPath, destf, fs2)) {
FileUtils.trashFilesUnderDir(fs2, oldPath, conf);
}
- if (inheritPerms) {
- inheritFromTable(tablePath, destf, conf, destFs);
- }
}
} catch (Exception e) {
//swallow the exception
@@ -2955,58 +2951,24 @@ private void constructOneLBLocationMap(FileStatus fSta,
}
}
- // rename src directory to destf
- if (srcs.length == 1 && srcs[0].isDir()) {
- // rename can fail if the parent doesn't exist
- Path destfp = destf.getParent();
- if (!destFs.exists(destfp)) {
- boolean success = destFs.mkdirs(destfp);
- if (!success) {
- LOG.warn("Error creating directory " + destf.toString());
- }
- if (inheritPerms && success) {
- inheritFromTable(tablePath, destfp, conf, destFs);
- }
- }
-
- // Copy/move each file under the source directory to avoid to delete the destination
- // directory if it is the root of an HDFS encryption zone.
- for (List<Path[]> sdpairs : result) {
- for (Path[] sdpair : sdpairs) {
- Path destParent = sdpair[1].getParent();
- FileSystem destParentFs = destParent.getFileSystem(conf);
- if (!destParentFs.isDirectory(destParent)) {
- boolean success = destFs.mkdirs(destParent);
- if (!success) {
- LOG.warn("Error creating directory " + destParent);
- }
- if (inheritPerms && success) {
- inheritFromTable(tablePath, destParent, conf, destFs);
- }
- }
- if (!moveFile(conf, sdpair[0], sdpair[1], true, isSrcLocal)) {
- throw new IOException("Unable to move file/directory from " + sdpair[0] +
- " to " + sdpair[1]);
- }
- }
- }
- } else { // srcf is a file or pattern containing wildcards
- if (!destFs.exists(destf)) {
- boolean success = destFs.mkdirs(destf);
- if (!success) {
- LOG.warn("Error creating directory " + destf.toString());
- }
- if (inheritPerms && success) {
- inheritFromTable(tablePath, destf, conf, destFs);
- }
- }
- // srcs must be a list of files -- ensured by LoadSemanticAnalyzer
- for (List<Path[]> sdpairs : result) {
- for (Path[] sdpair : sdpairs) {
- if (!moveFile(conf, sdpair[0], sdpair[1], true,
- isSrcLocal)) {
- throw new IOException("Error moving: " + sdpair[0] + " into: " + sdpair[1]);
- }
+ // first call FileUtils.mkdir to make sure that destf directory exists, if not, it creates
+ // destf with inherited permissions
+ boolean destfExist = FileUtils.mkdir(destFs, destf, true, conf);
+ if(!destfExist) {
+ throw new IOException("Directory " + destf.toString()
+ + " does not exist and could not be created.");
+ }
+
+ // Two cases:
+ // 1. srcs has only a src directory, if rename src directory to destf, we also need to
+ // Copy/move each file under the source directory to avoid to delete the destination
+ // directory if it is the root of an HDFS encryption zone.
+ // 2. srcs must be a list of files -- ensured by LoadSemanticAnalyzer
+ // in both cases, we move the file under destf
+ for (List<Path[]> sdpairs : result) {
+ for (Path[] sdpair : sdpairs) {
+ if (!moveFile(conf, sdpair[0], sdpair[1], true, isSrcLocal)) {
+ throw new IOException("Error moving: " + sdpair[0] + " into: " + sdpair[1]);
}
}
}
@@ -3015,38 +2977,6 @@ private void constructOneLBLocationMap(FileStatus fSta,
}
}
- /**
- * This method sets all paths from tablePath to destf (including destf) to have same permission as tablePath.
- * @param tablePath path of table
- * @param destf path of table-subdir.
- * @param conf
- * @param fs
- */
- private static void inheritFromTable(Path tablePath, Path destf, HiveConf conf, FileSystem fs) {
- if (!FileUtils.isSubDir(destf, tablePath, fs)) {
- //partition may not be under the parent.
- return;
- }
- HadoopShims shims = ShimLoader.getHadoopShims();
- //Calculate all the paths from the table dir, to destf
- //At end of this loop, currPath is table dir, and pathsToSet contain list of all those paths.
- Path currPath = destf;
- List<Path> pathsToSet = new LinkedList<Path>();
- while (!currPath.equals(tablePath)) {
- pathsToSet.add(currPath);
- currPath = currPath.getParent();
- }
-
- try {
- HadoopShims.HdfsFileStatus fullFileStatus = shims.getFullFileStatus(conf, fs, currPath);
- for (Path pathToSet : pathsToSet) {
- shims.setFullFileStatus(conf, fullFileStatus, fs, pathToSet);
- }
- } catch (Exception e) {
- LOG.warn("Error setting permissions or group of " + destf, e);
- }
- }
-
public static boolean isHadoop1() {
return ShimLoader.getMajorVersion().startsWith("0.20");
}