Repository: hive Updated Branches: refs/heads/master 107204a78 -> 38797d212
HIVE-13670 : Improve Beeline connect/reconnect semantics (Sushanth Sowmyan, reviewed by Thejas Nair) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/38797d21 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/38797d21 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/38797d21 Branch: refs/heads/master Commit: 38797d212fb1f4e9efa77f3002006dca6819219c Parents: 107204a Author: Sushanth Sowmyan <[email protected]> Authored: Thu May 12 01:09:36 2016 -0700 Committer: Sushanth Sowmyan <[email protected]> Committed: Thu May 12 01:09:36 2016 -0700 ---------------------------------------------------------------------- .../java/org/apache/hive/beeline/BeeLine.java | 10 +++ .../org/apache/hive/beeline/BeeLineOpts.java | 92 +++++++++++++++++--- .../java/org/apache/hive/beeline/Commands.java | 45 +++++++++- beeline/src/main/resources/BeeLine.properties | 1 + .../hive/beeline/TestBeeLineWithArgs.java | 85 ++++++++++++++++-- 5 files changed, 213 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/38797d21/beeline/src/java/org/apache/hive/beeline/BeeLine.java ---------------------------------------------------------------------- diff --git a/beeline/src/java/org/apache/hive/beeline/BeeLine.java b/beeline/src/java/org/apache/hive/beeline/BeeLine.java index 5e6e9ba..734eeb8 100644 --- a/beeline/src/java/org/apache/hive/beeline/BeeLine.java +++ b/beeline/src/java/org/apache/hive/beeline/BeeLine.java @@ -297,6 +297,12 @@ public class BeeLine implements Closeable { .withDescription("the JDBC URL to connect to") .create('u')); + // -r + options.addOption(OptionBuilder + .withLongOpt("reconnect") + .withDescription("Reconnect to last saved connect url (in conjunction with !save)") + .create('r')); + // -n <username> options.addOption(OptionBuilder .hasArg() @@ -739,6 +745,10 @@ public class BeeLine implements Closeable { pass = cl.getOptionValue("p"); } url = cl.getOptionValue("u"); + if ((url == null) && cl.hasOption("reconnect")){ + // If url was not specified with -u, but -r was present, use that. + url = getOpts().getLastConnectedUrl(); + } getOpts().setInitFiles(cl.getOptionValues("i")); getOpts().setScriptFile(cl.getOptionValue("f")); if (cl.getOptionValues('e') != null) { http://git-wip-us.apache.org/repos/asf/hive/blob/38797d21/beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java ---------------------------------------------------------------------- diff --git a/beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java b/beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java index 7a6ee5f..5aaa385 100644 --- a/beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java +++ b/beeline/src/java/org/apache/hive/beeline/BeeLineOpts.java @@ -28,6 +28,8 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; @@ -36,6 +38,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.TreeSet; import jline.Terminal; @@ -56,6 +59,8 @@ class BeeLineOpts implements Completer { public static final String DEFAULT_NULL_STRING = "NULL"; public static final char DEFAULT_DELIMITER_FOR_DSV = '|'; + public static String URL_ENV_PREFIX = "BEELINE_URL_"; + private final BeeLine beeLine; private boolean autosave = false; private boolean silent = false; @@ -102,6 +107,36 @@ class BeeLineOpts implements Completer { private Map<String, String> hiveConfVariables = new HashMap<String, String>(); private boolean helpAsked; + private String lastConnectedUrl = null; + + private TreeSet<String> cachedPropertyNameSet = null; + + @Retention(RetentionPolicy.RUNTIME) + public @interface Ignore { + // marker annotations for functions that Reflector should ignore / pretend it does not exist + + // NOTE: BeeLineOpts uses Reflector in an extensive way to call getters and setters on itself + // If you want to add any getters or setters to this class, but not have it interfere with + // saved variables in beeline.properties, careful use of this marker is needed. + // Also possible to get this by naming these functions obtainBlah instead of getBlah + // and so on, but that is not explicit and will likely surprise people looking at the + // code in the future. Better to be explicit in intent. + } + + public interface Env { + // Env interface to mock out dealing with Environment variables + // This allows us to interface with Environment vars through + // BeeLineOpts while allowing tests to mock out Env setting if needed. + String get(String envVar); + } + + public static Env env = new Env() { + @Override + public String get(String envVar) { + return System.getenv(envVar); // base env impl simply defers to System.getenv. + } + }; + public BeeLineOpts(BeeLine beeLine, Properties props) { this.beeLine = beeLine; if (terminal.getWidth() > 0) { @@ -177,24 +212,35 @@ class BeeLineOpts implements Completer { String[] propertyNames() throws IllegalAccessException, InvocationTargetException { - TreeSet<String> names = new TreeSet<String>(); + Set<String> names = propertyNamesSet(); // make sure we initialize if necessary + return names.toArray(new String[names.size()]); + } - // get all the values from getXXX methods - Method[] m = getClass().getDeclaredMethods(); - for (int i = 0; m != null && i < m.length; i++) { - if (!(m[i].getName().startsWith("get"))) { - continue; + Set<String> propertyNamesSet() + throws IllegalAccessException, InvocationTargetException { + if (cachedPropertyNameSet == null){ + TreeSet<String> names = new TreeSet<String>(); + + // get all the values from getXXX methods + Method[] m = getClass().getDeclaredMethods(); + for (int i = 0; m != null && i < m.length; i++) { + if (!(m[i].getName().startsWith("get"))) { + continue; + } + if (m[i].getAnnotation(Ignore.class) != null){ + continue; // not actually a getter + } + if (m[i].getParameterTypes().length != 0) { + continue; + } + String propName = m[i].getName().substring(3).toLowerCase(); + names.add(propName); } - if (m[i].getParameterTypes().length != 0) { - continue; - } - String propName = m[i].getName().substring(3).toLowerCase(); - names.add(propName); + cachedPropertyNameSet = names; } - return names.toArray(new String[names.size()]); + return cachedPropertyNameSet; } - public Properties toProperties() throws IllegalAccessException, InvocationTargetException, ClassNotFoundException { @@ -496,6 +542,7 @@ class BeeLineOpts implements Completer { return maxHeight; } + @Ignore public File getPropertiesFile() { return rcFile; } @@ -528,6 +575,7 @@ class BeeLineOpts implements Completer { this.nullEmptyString = nullStringEmpty; } + @Ignore public String getNullString(){ return nullEmptyString ? "" : DEFAULT_NULL_STRING; } @@ -567,5 +615,23 @@ class BeeLineOpts implements Completer { public boolean isHelpAsked() { return helpAsked; } + + public String getLastConnectedUrl(){ + return lastConnectedUrl; + } + + public void setLastConnectedUrl(String lastConnectedUrl){ + this.lastConnectedUrl = lastConnectedUrl; + } + + @Ignore + public static Env getEnv(){ + return env; + } + + @Ignore + public static void setEnv(Env envToUse){ + env = envToUse; + } } http://git-wip-us.apache.org/repos/asf/hive/blob/38797d21/beeline/src/java/org/apache/hive/beeline/Commands.java ---------------------------------------------------------------------- diff --git a/beeline/src/java/org/apache/hive/beeline/Commands.java b/beeline/src/java/org/apache/hive/beeline/Commands.java index 0178333..32c1275 100644 --- a/beeline/src/java/org/apache/hive/beeline/Commands.java +++ b/beeline/src/java/org/apache/hive/beeline/Commands.java @@ -36,6 +36,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.lang.reflect.Method; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLClassLoader; import java.sql.CallableStatement; @@ -310,7 +312,19 @@ public class Commands { public boolean reconnect(String line) { if (beeLine.getDatabaseConnection() == null || beeLine.getDatabaseConnection().getUrl() == null) { - return beeLine.error(beeLine.loc("no-current-connection")); + // First, let's try connecting using the last successful url - if that fails, then we error out. + String lastConnectedUrl = beeLine.getOpts().getLastConnectedUrl(); + if (lastConnectedUrl != null){ + Properties props = new Properties(); + props.setProperty("url",lastConnectedUrl); + try { + return connect(props); + } catch (IOException e) { + return beeLine.error(e); + } + } else { + return beeLine.error(beeLine.loc("no-current-connection")); + } } beeLine.info(beeLine.loc("reconnecting", beeLine.getDatabaseConnection().getUrl())); try { @@ -1299,7 +1313,8 @@ public class Commands { Properties props = new Properties(); if (url != null) { - props.setProperty("url", url); + String saveUrl = getUrlToUse(url); + props.setProperty("url", saveUrl); } if (driver != null) { props.setProperty("driver", driver); @@ -1314,6 +1329,31 @@ public class Commands { return connect(props); } + private String getUrlToUse(String urlParam) { + boolean useIndirectUrl = false; + // If the url passed to us is a valid url with a protocol, we use it as-is + // Otherwise, we assume it is a name of parameter that we have to get the url from + try { + URI tryParse = new URI(urlParam); + if (tryParse.getScheme() == null){ + // param had no scheme, so not a URL + useIndirectUrl = true; + } + } catch (URISyntaxException e){ + // param did not parse as a URL, so not a URL + useIndirectUrl = true; + } + if (useIndirectUrl){ + // Use url param indirectly - as the name of an env var that contains the url + // If the urlParam is "default", we would look for a BEELINE_URL_DEFAULT url + String envUrl = beeLine.getOpts().getEnv().get( + BeeLineOpts.URL_ENV_PREFIX + urlParam.toUpperCase()); + if (envUrl != null){ + return envUrl; + } + } + return urlParam; // default return the urlParam passed in as-is. + } private String getProperty(Properties props, String[] keys) { for (int i = 0; i < keys.length; i++) { @@ -1398,6 +1438,7 @@ public class Commands { beeLine.runInit(); beeLine.setCompletions(); + beeLine.getOpts().setLastConnectedUrl(url); return true; } catch (SQLException sqle) { beeLine.getDatabaseConnections().remove(); http://git-wip-us.apache.org/repos/asf/hive/blob/38797d21/beeline/src/main/resources/BeeLine.properties ---------------------------------------------------------------------- diff --git a/beeline/src/main/resources/BeeLine.properties b/beeline/src/main/resources/BeeLine.properties index bc40685..16f23a8 100644 --- a/beeline/src/main/resources/BeeLine.properties +++ b/beeline/src/main/resources/BeeLine.properties @@ -144,6 +144,7 @@ time-ms: ({0,number,#.###} seconds) cmd-usage: Usage: java org.apache.hive.cli.beeline.BeeLine \n \ \ -u <database url> the JDBC URL to connect to\n \ +\ -r reconnect to last saved connect url (in conjunction with !save)\n \ \ -n <username> the username to connect as\n \ \ -p <password> the password to connect as\n \ \ -d <driver class> the driver class to use\n \ http://git-wip-us.apache.org/repos/asf/hive/blob/38797d21/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java ---------------------------------------------------------------------- diff --git a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java index 8475a89..f9909ad 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java @@ -145,9 +145,9 @@ public class TestBeeLineWithArgs { } /** - * Attempt to execute a simple script file with the -f option to BeeLine - * Test for presence of an expected pattern - * in the output (stdout or stderr), fail if not found + * Attempt to execute a simple script file with the -f and -i option + * to BeeLine to test for presence of an expected pattern + * in the output (stdout or stderr), fail if not found. * Print PASSED or FAILED * @param expectedPattern Text to look for in command output/error * @param shouldMatch true if the pattern should be found, false if it should not @@ -155,6 +155,23 @@ public class TestBeeLineWithArgs { */ private void testScriptFile(String scriptText, String expectedPattern, boolean shouldMatch, List<String> argList) throws Throwable { + testScriptFile(scriptText, expectedPattern, shouldMatch, argList, true, true); + } + + /** + * Attempt to execute a simple script file with the -f or -i option + * to BeeLine (or both) to test for presence of an expected pattern + * in the output (stdout or stderr), fail if not found. + * Print PASSED or FAILED + * @param expectedPattern Text to look for in command output/error + * @param shouldMatch true if the pattern should be found, false if it should not + * @param testScript Whether we should test -f + * @param testInit Whether we should test -i + * @throws Exception on command execution error + */ + private void testScriptFile(String scriptText, String expectedPattern, + boolean shouldMatch, List<String> argList, + boolean testScript, boolean testInit) throws Throwable { // Put the script content in a temp file File scriptFile = File.createTempFile(this.getClass().getSimpleName(), "temp"); @@ -163,7 +180,7 @@ public class TestBeeLineWithArgs { os.print(scriptText); os.close(); - { + if (testScript) { List<String> copy = new ArrayList<String>(argList); copy.add("-f"); copy.add(scriptFile.getAbsolutePath()); @@ -177,7 +194,10 @@ public class TestBeeLineWithArgs { } } - { + // Not all scripts can be used as init scripts, so we parameterize. + // (scripts that test !connect, for eg., since -i runs after connects) + // So, we keep this optional. Most tests should leave this as true, however. + if (testInit) { List<String> copy = new ArrayList<String>(argList); copy.add("-i"); copy.add(scriptFile.getAbsolutePath()); @@ -768,4 +788,59 @@ public class TestBeeLineWithArgs { final String EXPECTED_PATTERN = "var1=value1"; testScriptFile(SCRIPT_TEXT, EXPECTED_PATTERN, true, argList); } + + /** + * Test Beeline !connect with beeline saved vars + * @throws Throwable + */ + @Test + public void testBeelineConnectEnvVar() throws Throwable { + final String jdbcUrl = miniHS2.getBaseJdbcURL(); + List<String> argList = new ArrayList<String>(); + argList.add("-u"); + argList.add("blue"); + argList.add("-d"); + argList.add(BeeLine.BEELINE_DEFAULT_JDBC_DRIVER); + + final String SCRIPT_TEXT = + "create table blueconnecttest (d int);\nshow tables;\n"; + final String EXPECTED_PATTERN = "blueconnecttest"; + + // We go through these hijinxes because java considers System.getEnv + // to be read-only, and offers no way to set an env var from within + // a process, only for processes that we sub-spawn. + + final BeeLineOpts.Env baseEnv = BeeLineOpts.getEnv(); + BeeLineOpts.Env newEnv = new BeeLineOpts.Env() { + @Override + public String get(String envVar) { + if (envVar.equalsIgnoreCase("BEELINE_URL_BLUE")){ + return jdbcUrl; + } else { + return baseEnv.get(envVar); + } + } + }; + BeeLineOpts.setEnv(newEnv); + + testScriptFile(SCRIPT_TEXT, EXPECTED_PATTERN, true, argList, true, false); + } + + /** + * Test that if we !close, we can still !reconnect + * @throws Throwable + */ + @Test + public void testBeelineReconnect() throws Throwable { + List<String> argList = getBaseArgs(miniHS2.getBaseJdbcURL()); + final String SCRIPT_TEXT = + "!close\n" + + "!reconnect\n\n\n" + + "create table reconnecttest (d int);\nshow tables;\n"; + final String EXPECTED_PATTERN = "reconnecttest"; + + testScriptFile(SCRIPT_TEXT, EXPECTED_PATTERN, true, argList, true, false); + + } + }
