Repository: hive Updated Branches: refs/heads/master d3e140f11 -> 3182378a5
HIVE-14259 : remove FileUtils.isSubDir() method (Zoltan Haindrich via Ashutosh Chauhan) Signed-off-by: Ashutosh Chauhan <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/3182378a Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/3182378a Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/3182378a Branch: refs/heads/master Commit: 3182378a52750896749cdf024119be27acda2afa Parents: d3e140f Author: Zoltan Haindrich <[email protected]> Authored: Sat Jul 16 03:40:00 2016 -0800 Committer: Ashutosh Chauhan <[email protected]> Committed: Wed Aug 3 22:08:23 2016 -0700 ---------------------------------------------------------------------- .../apache/hadoop/hive/common/FileUtils.java | 23 +----- .../hadoop/hive/common/TestFileUtils.java | 73 +++++++++++++++++++- .../apache/hadoop/hive/ql/metadata/Hive.java | 23 +++--- 3 files changed, 83 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/3182378a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java ---------------------------------------------------------------------- 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 3d3dddf..62c5504 100644 --- a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java +++ b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java @@ -627,25 +627,6 @@ public final class FileUtils { return result; } - /** - * Check if first path is a subdirectory of second path. - * Both paths must belong to the same filesystem. - * - * @param p1 first path - * @param p2 second path - * @param fs FileSystem, both paths must belong to the same filesystem - * @return - */ - public static boolean isSubDir(Path p1, Path p2, FileSystem fs) { - String path1 = fs.makeQualified(p1).toString(); - String path2 = fs.makeQualified(p2).toString(); - if (path1.startsWith(path2)) { - return true; - } - - return false; - } - public static boolean renameWithPerms(FileSystem fs, Path sourcePath, Path destPath, boolean inheritPerms, Configuration conf) throws IOException { @@ -688,7 +669,7 @@ public final class FileUtils { //FileSystem api doesn't have a .equals() function implemented, so using //the uri for comparison. FileSystem already uses uri+Configuration for //equality in its CACHE . - //Once equality has been added in HDFS-4321, we should make use of it + //Once equality has been added in HDFS-9159, we should make use of it return fs1.getUri().equals(fs2.getUri()); } @@ -893,7 +874,7 @@ public final class FileUtils { * @param candidates the candidate paths * @return */ - public static Path getParentRegardlessOfScheme(Path path, Set<Path> candidates) { + public static Path getParentRegardlessOfScheme(Path path, Collection<Path> candidates) { Path schemalessPath = Path.getPathWithoutSchemeAndAuthority(path); for(;path!=null && schemalessPath!=null; path=path.getParent(),schemalessPath=schemalessPath.getParent()){ http://git-wip-us.apache.org/repos/asf/hive/blob/3182378a/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java ---------------------------------------------------------------------- diff --git a/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java b/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java index e9fcc13..c02217a 100644 --- a/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java +++ b/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java @@ -18,10 +18,17 @@ package org.apache.hadoop.hive.common; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Set; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.hive.conf.HiveConf; import org.junit.Assert; import org.junit.Test; @@ -31,12 +38,74 @@ import org.slf4j.LoggerFactory; import com.google.common.collect.Sets; import com.google.common.io.Files; -import junit.framework.TestCase; +public class TestFileUtils { -public class TestFileUtils extends TestCase { public static final Logger LOG = LoggerFactory.getLogger(TestFileUtils.class); @Test + public void isPathWithinSubtree_samePrefix() { + Path path = new Path("/somedir1"); + Path subtree = new Path("/somedir"); + + assertFalse(FileUtils.isPathWithinSubtree(path, subtree)); + } + + @Test + public void isPathWithinSubtree_rootIsInside() { + Path path = new Path("/foo"); + Path subtree = new Path("/foo"); + assertTrue(FileUtils.isPathWithinSubtree(path, subtree)); + } + + @Test + public void isPathWithinSubtree_descendantInside() { + Path path = new Path("/foo/bar"); + Path subtree = new Path("/foo"); + assertTrue(FileUtils.isPathWithinSubtree(path, subtree)); + } + + @Test + public void isPathWithinSubtree_relativeWalk() { + Path path = new Path("foo/../../bar"); + Path subtree = new Path("../bar"); + assertTrue(FileUtils.isPathWithinSubtree(path, subtree)); + } + + @Test + public void getParentRegardlessOfScheme_badCases() { + Path path = new Path("proto://host1/foo/bar/baz"); + ArrayList<Path> candidates = new ArrayList<>(); + candidates.add(new Path("badproto://host1/foo")); + candidates.add(new Path("proto://badhost1/foo")); + candidates.add(new Path("proto://host1:71/foo/bar/baz")); + candidates.add(new Path("proto://host1/badfoo")); + candidates.add(new Path("/badfoo")); + Path res = FileUtils.getParentRegardlessOfScheme(path, candidates); + assertNull("none of these paths may match", res); + } + + @Test + public void getParentRegardlessOfScheme_priority() { + Path path = new Path("proto://host1/foo/bar/baz"); + ArrayList<Path> candidates = new ArrayList<>(); + Path expectedPath; + candidates.add(new Path("proto://host1/")); + candidates.add(expectedPath = new Path("proto://host1/foo")); + Path res = FileUtils.getParentRegardlessOfScheme(path, candidates); + assertEquals(expectedPath, res); + } + + @Test + public void getParentRegardlessOfScheme_root() { + Path path = new Path("proto://host1/foo"); + ArrayList<Path> candidates = new ArrayList<>(); + Path expectedPath; + candidates.add(expectedPath = new Path("proto://host1/foo")); + Path res = FileUtils.getParentRegardlessOfScheme(path, candidates); + assertEquals(expectedPath, res); + } + + @Test public void testGetJarFilesByPath() { HiveConf conf = new HiveConf(this.getClass()); File tmpDir = Files.createTempDir(); http://git-wip-us.apache.org/repos/asf/hive/blob/3182378a/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 57433bb..deaaac4 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 @@ -2796,8 +2796,8 @@ private void constructOneLBLocationMap(FileStatus fSta, return false; } - String fullF1 = getQualifiedPathWithoutSchemeAndAuthority(srcf, srcFs) + Path.SEPARATOR; - String fullF2 = getQualifiedPathWithoutSchemeAndAuthority(destf, destFs) + Path.SEPARATOR; + String fullF1 = getQualifiedPathWithoutSchemeAndAuthority(srcf, srcFs).toString() + Path.SEPARATOR; + String fullF2 = getQualifiedPathWithoutSchemeAndAuthority(destf, destFs).toString() + Path.SEPARATOR; boolean isInTest = HiveConf.getBoolVar(srcFs.getConf(), ConfVars.HIVE_IN_TEST); // In the automation, the data warehouse is the local file system based. @@ -2827,10 +2827,10 @@ private void constructOneLBLocationMap(FileStatus fSta, return fullF1.startsWith(fullF2); } - private static String getQualifiedPathWithoutSchemeAndAuthority(Path srcf, FileSystem fs) { + private static Path getQualifiedPathWithoutSchemeAndAuthority(Path srcf, FileSystem fs) { Path currentWorkingDir = fs.getWorkingDirectory(); Path path = srcf.makeQualified(srcf.toUri(), currentWorkingDir); - return ShimLoader.getHadoopShims().getPathWithoutSchemeAndAuthority(path).toString(); + return ShimLoader.getHadoopShims().getPathWithoutSchemeAndAuthority(path); } private static Path mvFile(HiveConf conf, Path srcf, Path destf, boolean isSrcLocal, @@ -2859,10 +2859,8 @@ private void constructOneLBLocationMap(FileStatus fSta, FileSystem destFS = dest.getFileSystem(conf); FileSystem srcFS = src.getFileSystem(conf); if (isSubDir(src, dest, srcFS, destFS, isSrcLocal)) { - final Path fullSrcPath = new Path( - getQualifiedPathWithoutSchemeAndAuthority(src, srcFS)); - final Path fullDestPath = new Path( - getQualifiedPathWithoutSchemeAndAuthority(dest, destFS)); + final Path fullSrcPath = getQualifiedPathWithoutSchemeAndAuthority(src, srcFS); + final Path fullDestPath = getQualifiedPathWithoutSchemeAndAuthority(dest, destFS); if (fullSrcPath.equals(fullDestPath)) { return; } @@ -3217,18 +3215,17 @@ private void constructOneLBLocationMap(FileStatus fSta, boolean isOldPathUnderDestf = false; FileStatus[] statuses = null; try { - FileSystem fs2 = oldPath.getFileSystem(conf); - statuses = fs2.listStatus(oldPath, FileUtils.HIDDEN_FILES_PATH_FILTER); + FileSystem oldFs = oldPath.getFileSystem(conf); + statuses = oldFs.listStatus(oldPath, FileUtils.HIDDEN_FILES_PATH_FILTER); // Do not delete oldPath if: // - destf is subdir of oldPath - //if ( !(fs2.equals(destf.getFileSystem(conf)) && FileUtils.isSubDir(oldPath, destf, fs2))) - isOldPathUnderDestf = FileUtils.isSubDir(oldPath, destf, fs2); + isOldPathUnderDestf = isSubDir(oldPath, destf, oldFs, destFs, false); if (isOldPathUnderDestf) { // if oldPath is destf or its subdir, its should definitely be deleted, otherwise its // existing content might result in incorrect (extra) data. // But not sure why we changed not to delete the oldPath in HIVE-8750 if it is // not the destf or its subdir? - oldPathDeleted = trashFiles(fs2, statuses, conf); + oldPathDeleted = trashFiles(oldFs, statuses, conf); } } catch (IOException e) { if (isOldPathUnderDestf) {
