IMPALA-5416: Fix an impala-shell command recursion bug Impala-shell crashes with 2 source commands on the same line and runs a command multiple times if it shares the same line with a source command. The bug is caused by a misuse of cmdqueue. The cmdqueue member of cmd.Cmd is used to execute commands not directly from user input in an event loop. When a 'source' is run, execute_query_list() is called which also executes the commands in cmdqueue, causing them to be executed twice. The fix is for execute_query_list() to not run the commands in cmdqueue. For the non-interactive case, where the event loop won't be run, we call execute_query_list() with cmdqueue so that the commands get run. A test case is added to test_shell_interactive.py.
Change-Id: I453af2d4694d47e184031cb07ecd2af259ba20f3 Reviewed-on: http://gerrit.cloudera.org:8080/8063 Reviewed-by: Alex Behm <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/bd08ed42 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/bd08ed42 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/bd08ed42 Branch: refs/heads/master Commit: bd08ed42304b7375f5bc9c5ba6d72088cc76678a Parents: a643854 Author: Tianyi Wang <[email protected]> Authored: Wed Sep 13 18:25:11 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Sep 21 21:41:31 2017 +0000 ---------------------------------------------------------------------- shell/impala_shell.py | 6 ++++-- tests/shell/test_shell_interactive.py | 8 ++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/bd08ed42/shell/impala_shell.py ---------------------------------------------------------------------- diff --git a/shell/impala_shell.py b/shell/impala_shell.py index 6d9df4f..8db3e43 100755 --- a/shell/impala_shell.py +++ b/shell/impala_shell.py @@ -1208,7 +1208,7 @@ class ImpalaShell(object, cmd.Cmd): if not self.imp_client.connected: print_to_stderr('Not connected to Impala, could not execute queries.') return False - queries = [ self.sanitise_input(q) for q in self.cmdqueue + queries ] + queries = [self.sanitise_input(q) for q in queries] for q in queries: if self.onecmd(q) is CmdStatus.ERROR: print_to_stderr('Could not execute command: %s' % q) @@ -1321,7 +1321,9 @@ def execute_queries_non_interactive_mode(options): return queries = parse_query_text(query_text) - if not ImpalaShell(options).execute_query_list(queries): + shell = ImpalaShell(options) + if not (shell.execute_query_list(shell.cmdqueue) and + shell.execute_query_list(queries)): sys.exit(1) if __name__ == "__main__": http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/bd08ed42/tests/shell/test_shell_interactive.py ---------------------------------------------------------------------- diff --git a/tests/shell/test_shell_interactive.py b/tests/shell/test_shell_interactive.py index 7b85a55..c37c2be 100755 --- a/tests/shell/test_shell_interactive.py +++ b/tests/shell/test_shell_interactive.py @@ -294,14 +294,18 @@ class TestImpalaShellInteractive(object): try: # Change working dir so that SOURCE command in shell.cmds can find shell2.cmds. os.chdir("%s/tests/shell/" % os.environ['IMPALA_HOME']) - result = run_impala_shell_interactive("source shell.cmds;") + # IMPALA-5416: Test that a command following 'source' won't be run twice. + result = run_impala_shell_interactive("source shell.cmds;select \"second command\";") assert "Query: use FUNCTIONAL" in result.stderr assert "Query: show TABLES" in result.stderr assert "alltypes" in result.stdout - # This is from shell2.cmds, the result of sourcing a file from a sourced file. assert "select VERSION()" in result.stderr assert "version()" in result.stdout + assert len(re.findall("'second command'", result.stdout)) == 1 + # IMPALA-5416: Test that two source commands on a line won't crash the shell. + result = run_impala_shell_interactive("source shell.cmds;source shell.cmds;") + assert len(re.findall("version\(\)", result.stdout)) == 2 finally: os.chdir(cwd)
