Attila Jeges has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15233 )

Change subject: IMPALA-9036: Fix CTRL+C a multiline query in impala-shell
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/15233/2/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/15233/2/tests/shell/test_shell_interactive.py@252
PS2, Line 252:        "wrong\n1", "[1]: incorrect\n2",
             :         "select 3 --comment\n", "[2]: select 4 --comment",
             :         "select 5 --comment\n\n\n", "[3]: select 6 --comment",
             :         "select /*comment*/\n7", "[4]: select /*comment*/\n8",
             :         "select\n/*comm\nent*/\n9", "[5]: 
select\n/*comm\nent*/\n10"
I'd use input lines like "line 1", "line 2', "one", "two" or something similar, 
with newlines scattered through. You can throw in some SQL- or C-style comments 
too for good measure but keep them short.

In general the idea is to make these input lines as simple as possible to make 
the intent clear, which is that we expect that these erroneous lines will be 
ignored because of Ctrl-C.


http://gerrit.cloudera.org:8080/#/c/15233/2/tests/shell/test_shell_interactive.py@256
PS2, Line 256: "[5]: select\n/*comm\nent*/\n10"
I think there are two test cases here that we should address:
1. When the last line before Ctrl-C ends with newline.
2. When it doesn't.


http://gerrit.cloudera.org:8080/#/c/15233/2/tests/shell/test_shell_interactive.py@259
PS2, Line 259: child_proc.sendintr()
If the very last line before Ctrl-C ends with a newline, you should add before 
L259 a child_proc.expect('     >') to make it clear that impala-shell is 
waiting for more input.



--
To view, visit http://gerrit.cloudera.org:8080/15233
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id8d8bdaee929e2655eb66e886ae92a02d3fbd83f
Gerrit-Change-Number: 15233
Gerrit-PatchSet: 2
Gerrit-Owner: Adam Tamas <[email protected]>
Gerrit-Reviewer: Adam Tamas <[email protected]>
Gerrit-Reviewer: Attila Jeges <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Wed, 19 Feb 2020 10:35:41 +0000
Gerrit-HasComments: Yes

Reply via email to