You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ct...@apache.org on 2016/11/10 00:54:28 UTC
hive git commit: HIVE-12891: Hive fails when java.io.tmpdir is set to
a relative location (Barna Zsombor Klara via Chaoyu Tang)
Repository: hive
Updated Branches:
refs/heads/master 34e5a7d2f -> e7aaf9678
HIVE-12891: Hive fails when java.io.tmpdir is set to a relative location (Barna Zsombor Klara via Chaoyu Tang)
Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/e7aaf967
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/e7aaf967
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/e7aaf967
Branch: refs/heads/master
Commit: e7aaf96780041de096d5260abe80ca50e321f9e3
Parents: 34e5a7d
Author: Chaoyu Tang <ct...@cloudera.com>
Authored: Wed Nov 9 19:54:04 2016 -0500
Committer: Chaoyu Tang <ct...@cloudera.com>
Committed: Wed Nov 9 19:54:04 2016 -0500
----------------------------------------------------------------------
.../apache/hadoop/hive/common/FileUtils.java | 8 ++
.../hadoop/hive/conf/SystemVariables.java | 43 ++++++++---
.../JavaIOTmpdirVariableCoercion.java | 63 ++++++++++++++++
.../hive/conf/valcoersion/VariableCoercion.java | 42 +++++++++++
.../hadoop/hive/common/TestFileUtils.java | 16 +++-
.../hadoop/hive/conf/TestSystemVariables.java | 77 ++++++++++++++++++++
6 files changed, 237 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hive/blob/e7aaf967/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 1d734f9..1d8c041 100644
--- a/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
+++ b/common/src/java/org/apache/hadoop/hive/common/FileUtils.java
@@ -560,6 +560,14 @@ public final class FileUtils {
}
}
+ public static Path makeAbsolute(FileSystem fileSystem, Path path) throws IOException {
+ if (path.isAbsolute()) {
+ return path;
+ } else {
+ return new Path(fileSystem.getWorkingDirectory(), path);
+ }
+ }
+
/**
* Copies files between filesystems.
*/
http://git-wip-us.apache.org/repos/asf/hive/blob/e7aaf967/common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java b/common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java
index 9f59f11..14b4afe 100644
--- a/common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java
+++ b/common/src/java/org/apache/hadoop/hive/conf/SystemVariables.java
@@ -17,10 +17,14 @@
*/
package org.apache.hadoop.hive.conf;
+import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import com.google.common.collect.ImmutableMap;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.conf.valcoersion.JavaIOTmpdirVariableCoercion;
+import org.apache.hadoop.hive.conf.valcoersion.VariableCoercion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -28,6 +32,11 @@ public class SystemVariables {
private static final Logger l4j = LoggerFactory.getLogger(SystemVariables.class);
protected static Pattern varPat = Pattern.compile("\\$\\{[^\\}\\$\u0020]+\\}");
+ private static final SystemVariables INSTANCE = new SystemVariables();
+ private static final Map<String, VariableCoercion> COERCIONS =
+ ImmutableMap.<String, VariableCoercion>builder()
+ .put(JavaIOTmpdirVariableCoercion.INSTANCE.getName(), JavaIOTmpdirVariableCoercion.INSTANCE)
+ .build();
public static final String ENV_PREFIX = "env:";
public static final String SYSTEM_PREFIX = "system:";
@@ -36,22 +45,34 @@ public class SystemVariables {
public static final String METACONF_PREFIX = "metaconf:";
public static final String SET_COLUMN_NAME = "set";
- protected String getSubstitute(Configuration conf, String var) {
- String val = null;
+ protected String getSubstitute(Configuration conf, String variableName) {
try {
- if (var.startsWith(SYSTEM_PREFIX)) {
- val = System.getProperty(var.substring(SYSTEM_PREFIX.length()));
+ if (variableName.startsWith(SYSTEM_PREFIX)) {
+ String propertyName = variableName.substring(SYSTEM_PREFIX.length());
+ String originalValue = System.getProperty(propertyName);
+ return applyCoercion(variableName, originalValue);
}
} catch(SecurityException se) {
l4j.warn("Unexpected SecurityException in Configuration", se);
}
- if (val == null && var.startsWith(ENV_PREFIX)) {
- val = System.getenv(var.substring(ENV_PREFIX.length()));
+
+ if (variableName.startsWith(ENV_PREFIX)) {
+ return System.getenv(variableName.substring(ENV_PREFIX.length()));
+ }
+
+ if (conf != null && variableName.startsWith(HIVECONF_PREFIX)) {
+ return conf.get(variableName.substring(HIVECONF_PREFIX.length()));
}
- if (val == null && conf != null && var.startsWith(HIVECONF_PREFIX)) {
- val = conf.get(var.substring(HIVECONF_PREFIX.length()));
+
+ return null;
+ }
+
+ private String applyCoercion(String variableName, String originalValue) {
+ if (COERCIONS.containsKey(variableName)) {
+ return COERCIONS.get(variableName).getCoerced(originalValue);
+ } else {
+ return originalValue;
}
- return val;
}
public static boolean containsVar(String expr) {
@@ -59,11 +80,11 @@ public class SystemVariables {
}
static String substitute(String expr) {
- return expr == null ? null : new SystemVariables().substitute(null, expr, 1);
+ return expr == null ? null : INSTANCE.substitute(null, expr, 1);
}
static String substitute(Configuration conf, String expr) {
- return expr == null ? null : new SystemVariables().substitute(conf, expr, 1);
+ return expr == null ? null : INSTANCE.substitute(conf, expr, 1);
}
protected final String substitute(Configuration conf, String expr, int depth) {
http://git-wip-us.apache.org/repos/asf/hive/blob/e7aaf967/common/src/java/org/apache/hadoop/hive/conf/valcoersion/JavaIOTmpdirVariableCoercion.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/valcoersion/JavaIOTmpdirVariableCoercion.java b/common/src/java/org/apache/hadoop/hive/conf/valcoersion/JavaIOTmpdirVariableCoercion.java
new file mode 100644
index 0000000..b25b8f6
--- /dev/null
+++ b/common/src/java/org/apache/hadoop/hive/conf/valcoersion/JavaIOTmpdirVariableCoercion.java
@@ -0,0 +1,63 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.conf.valcoersion;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocalFileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hive.common.FileUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
+import java.io.IOException;
+
+/**
+ * Enforces absolute paths to be used for the java.io.tmpdir system variable.
+ * @see VariableCoercion
+ * @see org.apache.hadoop.hive.conf.SystemVariables
+ */
+public class JavaIOTmpdirVariableCoercion extends VariableCoercion {
+ private static final Log LOG = LogFactory.getLog(JavaIOTmpdirVariableCoercion.class);
+ private static final String NAME = "system:java.io.tmpdir";
+ private static final FileSystem LOCAL_FILE_SYSTEM = new LocalFileSystem();
+
+ public static final JavaIOTmpdirVariableCoercion INSTANCE = new JavaIOTmpdirVariableCoercion();
+
+ private JavaIOTmpdirVariableCoercion() {
+ super(NAME);
+ }
+
+ private String coerce(String originalValue) {
+ if (originalValue == null || originalValue.isEmpty()) return originalValue;
+
+ try {
+ Path originalPath = new Path(originalValue);
+ Path absolutePath = FileUtils.makeAbsolute(LOCAL_FILE_SYSTEM, originalPath);
+ return absolutePath.toString();
+ } catch (IOException exception) {
+ LOG.warn(String.format("Unable to resolve 'java.io.tmpdir' for absolute path '%s'", originalValue));
+ return originalValue;
+ }
+ }
+
+ @Override
+ public String getCoerced(String originalValue) {
+ return coerce(originalValue);
+ }
+
+}
http://git-wip-us.apache.org/repos/asf/hive/blob/e7aaf967/common/src/java/org/apache/hadoop/hive/conf/valcoersion/VariableCoercion.java
----------------------------------------------------------------------
diff --git a/common/src/java/org/apache/hadoop/hive/conf/valcoersion/VariableCoercion.java b/common/src/java/org/apache/hadoop/hive/conf/valcoersion/VariableCoercion.java
new file mode 100644
index 0000000..eaa9843
--- /dev/null
+++ b/common/src/java/org/apache/hadoop/hive/conf/valcoersion/VariableCoercion.java
@@ -0,0 +1,42 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.conf.valcoersion;
+
+import org.apache.hadoop.conf.Configuration;
+
+/**
+ * VariableCoercions are used to enforce rules related to system variables.
+ * These rules may transform the value of system properties returned by the
+ * {@link org.apache.hadoop.hive.conf.SystemVariables SystemVariables} utility class
+ */
+public abstract class VariableCoercion {
+ private final String name;
+
+ public VariableCoercion(String name) {
+ this.name = name;
+ }
+
+ public String getName() { return this.name; }
+
+ /**
+ * Coerce the original value of the variable
+ * @param originalValue the unmodified value
+ * @return transformed value
+ */
+ public abstract String getCoerced(String originalValue);
+}
http://git-wip-us.apache.org/repos/asf/hive/blob/e7aaf967/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 c02217a..d82c531 100644
--- a/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java
+++ b/common/src/test/org/apache/hadoop/hive/common/TestFileUtils.java
@@ -28,6 +28,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Set;
+import org.apache.hadoop.fs.LocalFileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.conf.HiveConf;
import org.junit.Assert;
@@ -115,7 +116,7 @@ public class TestFileUtils {
try {
org.apache.commons.io.FileUtils.touch(jarFile1);
Set<String> jars = FileUtils.getJarFilesByPath(tmpDir.getAbsolutePath(), conf);
- Assert.assertEquals(Sets.newHashSet("file://" + jarFileName1),jars);
+ Assert.assertEquals(Sets.newHashSet("file://" + jarFileName1), jars);
jars = FileUtils.getJarFilesByPath("/folder/not/exist", conf);
Assert.assertTrue(jars.isEmpty());
@@ -132,4 +133,17 @@ public class TestFileUtils {
org.apache.commons.io.FileUtils.deleteQuietly(tmpDir);
}
}
+
+ @Test
+ public void testRelativePathToAbsolutePath() throws IOException {
+ LocalFileSystem localFileSystem = new LocalFileSystem();
+ Path actualPath = FileUtils.makeAbsolute(localFileSystem, new Path("relative/path"));
+ Path expectedPath = new Path(localFileSystem.getWorkingDirectory(), "relative/path");
+ assertEquals(expectedPath.toString(), actualPath.toString());
+
+ Path absolutePath = new Path("/absolute/path");
+ Path unchangedPath = FileUtils.makeAbsolute(localFileSystem, new Path("/absolute/path"));
+
+ assertEquals(unchangedPath.toString(), absolutePath.toString());
+ }
}
http://git-wip-us.apache.org/repos/asf/hive/blob/e7aaf967/common/src/test/org/apache/hadoop/hive/conf/TestSystemVariables.java
----------------------------------------------------------------------
diff --git a/common/src/test/org/apache/hadoop/hive/conf/TestSystemVariables.java b/common/src/test/org/apache/hadoop/hive/conf/TestSystemVariables.java
new file mode 100644
index 0000000..e8dd632
--- /dev/null
+++ b/common/src/test/org/apache/hadoop/hive/conf/TestSystemVariables.java
@@ -0,0 +1,77 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.conf;
+
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.LocalFileSystem;
+import org.apache.hadoop.fs.Path;
+import org.junit.Test;
+
+import static junit.framework.TestCase.assertEquals;
+import static junit.framework.TestCase.assertNull;
+
+public class TestSystemVariables {
+ public static final String SYSTEM = "system";
+
+ private String makeVarName(String prefix, String value) {
+ return String.format("${%s:%s}", prefix, value);
+ }
+
+ @Test
+ public void test_RelativeJavaIoTmpDir_CoercedTo_AbsolutePath() {
+ FileSystem localFileSystem = new LocalFileSystem();
+ String systemJavaIoTmpDir = makeVarName(SYSTEM, "java.io.tmpdir");
+
+ System.setProperty("java.io.tmpdir", "./relativePath");
+ Path relativePath = new Path(localFileSystem.getWorkingDirectory(), "./relativePath");
+ assertEquals(relativePath.toString(), SystemVariables.substitute(systemJavaIoTmpDir));
+
+ System.setProperty("java.io.tmpdir", "this/is/a/relative/path");
+ Path thisIsARelativePath= new Path(localFileSystem.getWorkingDirectory(), "this/is/a/relative/path");
+ assertEquals(thisIsARelativePath.toString(), SystemVariables.substitute(systemJavaIoTmpDir));
+ }
+
+ @Test
+ public void test_AbsoluteJavaIoTmpDir_NotChanged() {
+ FileSystem localFileSystem = new LocalFileSystem();
+ String systemJavaIoTmpDir = makeVarName(SYSTEM, "java.io.tmpdir");
+
+ System.setProperty("java.io.tmpdir", "file:/this/is/an/absolute/path");
+ Path absolutePath = new Path("file:/this/is/an/absolute/path");
+ assertEquals(absolutePath.toString(), SystemVariables.substitute(systemJavaIoTmpDir));
+ }
+
+ @Test
+ public void test_RelativePathWithNoCoercion_NotChanged() {
+ FileSystem localFileSystem = new LocalFileSystem();
+ String systemJavaIoTmpDir = makeVarName(SYSTEM, "java.io._NOT_tmpdir");
+
+ System.setProperty("java.io._NOT_tmpdir", "this/is/an/relative/path");
+ Path relativePath = new Path("this/is/an/relative/path");
+ assertEquals(relativePath.toString(), SystemVariables.substitute(systemJavaIoTmpDir));
+ }
+
+ @Test
+ public void test_EmptyJavaIoTmpDir_NotChanged() {
+ FileSystem localFileSystem = new LocalFileSystem();
+ String systemJavaIoTmpDir = makeVarName(SYSTEM, "java.io.tmpdir");
+
+ System.setProperty("java.io.tmpdir", "");
+ assertEquals("", SystemVariables.substitute(systemJavaIoTmpDir));
+ }
+}