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 9: Code-Review+1 (3 comments) I'll let someone else +2 this since the approach is debatable. The patch looks good to me once the remaining nits are addressed. http://gerrit.cloudera.org:8080/#/c/9933/7/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9933/7/shell/impala_shell.py@1306 PS7, Line 1306: args: connect > Done. docstring updated. In that case, we could probably contribute it to the argparse, I already see a couple of your commits there :-). http://gerrit.cloudera.org:8080/#/c/9933/9/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9933/9/shell/impala_shell.py@326 PS9, Line 326: command, nit: move these to the previous line. http://gerrit.cloudera.org:8080/#/c/9933/9/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/9933/9/tests/shell/test_shell_interactive.py@547 PS9, Line 547: assert ('--delete\n', 'select 1') == \ Sorry missed this the last time, could you add an example with multiple prefix comments. /* foo */ /* bar */ -- test select 1 -- 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: 9 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, 30 Apr 2018 23:34:07 +0000 Gerrit-HasComments: Yes
