impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-2640: Make a given command case-sensitive
Date Tue, 12 Dec 2017 23:31:06 GMT
Tim Armstrong has posted comments on this change. ( )

Change subject: IMPALA-2640: Make a given command case-sensitive

Patch Set 6:

Commit Message:
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
File shell/
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.
File shell/
PS9, Line 305:                                                  self.set_query_options)
Long line
PS9, Line 310:            ! <cmd>
Long line.
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.
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
To unsubscribe, visit

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 <>
Gerrit-Reviewer: Andre Araujo <>
Gerrit-Reviewer: John Russell <>
Gerrit-Reviewer: Kim Jin Chul <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: Zoltan Borok-Nagy <>
Gerrit-Comment-Date: Tue, 12 Dec 2017 23:31:06 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message