Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8762 )
Change subject: IMPALA-2640: Make a given command case-sensitive ...................................................................... Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/8762/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8762/6//COMMIT_MSG@7 PS6, Line 7: IMPALA-2640: Make a given command case-sensitive Unless I'm missing something, doesn't this solve IMPALA-4664 too? I checked out this commit and played around and I get: [localhost:21000] > sElect'FOO'; Query: sElect 'FOO' Query submitted at: 2017-12-12 15:30:44 (Coordinator: http://tarmstrong-box:25000) Query progress can be monitored at: http://tarmstrong-box:25000/query_plan?query_id=c049cc68f0842616:4419587c00000000 +-------+ | 'foo' | +-------+ | FOO | +-------+ Fetched 1 row(s) in 0.00s http://gerrit.cloudera.org:8080/#/c/8762/6/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8762/6/shell/impala_shell.py@572 PS6, Line 572: return func(arg, cmd_) > Done. It's a matter of taste but I think passing in cmd as an argument is clearer - fewer member variables and implicit argument parsing seems easier to follow. I agree it still feels a bit weird in some ways. Anyway, I think I'm ok with the current version. http://gerrit.cloudera.org:8080/#/c/8762/9/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/8762/9/shell/impala_shell.py@305 PS9, Line 305: self.set_query_options) Long line http://gerrit.cloudera.org:8080/#/c/8762/9/shell/impala_shell.py@310 PS9, Line 310: ! <cmd> Long line. http://gerrit.cloudera.org:8080/#/c/8762/9/shell/impala_shell.py@555 PS9, Line 555: Can you include attribution for where the copied code came from (e.g. see be/src/runtime/string-search.h). It also needs to be added to LICENSE.txt. It looks like it's licensed with the PSF license V2, which we already include in LICENSE.txt. http://gerrit.cloudera.org:8080/#/c/8762/9/shell/impala_shell.py@571 PS9, Line 571: etur _ on the end of a local variable is a bit weird to me - can we call it command instead? -- To view, visit http://gerrit.cloudera.org:8080/8762 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifdce9781d1d97596c188691b62a141b9bd137610 Gerrit-Change-Number: 8762 Gerrit-PatchSet: 6 Gerrit-Owner: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Andre Araujo <ara...@cloudera.com> Gerrit-Reviewer: John Russell <jruss...@cloudera.com> Gerrit-Reviewer: Kim Jin Chul <jinc...@gmail.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 12 Dec 2017 23:31:06 +0000 Gerrit-HasComments: Yes
