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 <fwij...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Apr 2018 19:39:15 +0000
Gerrit-HasComments: Yes

Reply via email to