[Impala-ASF-CR] IMPALA-4664: Unexpected string conversion in Shell

["Tim Armstrong (Code Review)" <ge...@cloudera.org>]

Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8639 )

Change subject: IMPALA-4664: Unexpected string conversion in Shell
......................................................................


Patch Set 4:

(1 comment)

I had a suggestion for changing the approach after I spent some time understanding the change. Let me know what you think.

http://gerrit.cloudera.org:8080/#/c/8639/4/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/8639/4/shell/impala_shell.py@337
PS4, Line 337:   def sanitise_input(self, args):
I feel like all this string manipulation is very hard to reason about, leading to bugs like this.

It seems like if we're calling sqlparse.parse() to construct Statement instances, we should return those Statements, which will be easier to operate on, instead of converting it back to strings.

Instead we're translating tokens back to strings, joining then, then resplitting them with sqlparse.split().



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba6fa4e9ac570d22135ba51b844db8f9be965ca3
Gerrit-Change-Number: 8639
Gerrit-PatchSet: 4
Gerrit-Owner: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Kim Jin Chul <ji...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <za...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Dec 2017 00:42:21 +0000
Gerrit-HasComments: Yes