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 <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Kim Jin Chul <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zach Amsden <[email protected]> Gerrit-Comment-Date: Thu, 07 Dec 2017 00:42:21 +0000 Gerrit-HasComments: Yes
