Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/9933 )
Change subject: IMPALA-2195: Improper handling of comments in queries ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/9933/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9933/3/shell/impala_shell.py@1276 PS3, Line 1276: trailing comment, the trailing You mean leading/prefix comments? http://gerrit.cloudera.org:8080/#/c/9933/3/shell/impala_shell.py@1279 PS3, Line 1279: line = ImpalaShell.strip_trailing_comment(line.strip()).encode('utf-8') I think we should not overwrite line by stripping comments totally. Since 'line' is what is sent to the coordinator, we should preserve the leading comments and let the frontend parser deal with them. We should be able to deduce the 'command' part from a temporarily stripped query. Thoughts? http://gerrit.cloudera.org:8080/#/c/9933/3/shell/impala_shell.py@1285 PS3, Line 1285: def strip_trailing_comment(sql): Just wanted to check if you considered Alex's idea of reusing a stripped query stmt by storing it in the shell state? We are are parsing the query multiple times on the client side and it is expensive for large queries. We can as well fix that? -- To view, visit http://gerrit.cloudera.org:8080/9933 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ac7cb5a30e6dda73ebe761d9f0eb9ba038e14a7 Gerrit-Change-Number: 9933 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Comment-Date: Mon, 09 Apr 2018 19:39:15 +0000 Gerrit-HasComments: Yes
