impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tianyi Wang (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-992: Rerun past queries from history in shell
Date Tue, 15 Aug 2017 21:36:35 GMT
Tianyi Wang has posted comments on this change.

Change subject: IMPALA-992: Rerun past queries from history in shell

Patch Set 4:

Commit Message:

PS2, Line 7: IMPALA
> nit: capitalize

PS2, Line 9: @
> What's the reason for picking "@" over "!"? The latter is used in shells an
Currently "!" is used to run shell commands in impala-shell.

PS2, Line 12: _length, -1]. Negavie val
> Can you give a usage example? Does the history count backwards or forwards 
More example and explanation is given.
File shell/

PS2, Line 79: object
> Why is this needed?
The direct reason is that "super" in line 1164 cannot be used with an old style class. And
old style class is not recommended after python 2.1.

Line 1073:     print_to_stderr("The readline module was either not found or disabled. Command
> nit: You can remove the else and unindent the block, just like you do in th

PS2, Line 1080: one' a
> Not your change, but:

PS2, Line 1090: 
> Format on two lines to make it easier to read.

PS2, Line 1102:     if not (0 < cmd_idx <= history_length or -history_length <= cmd_idx
< 0):
> Please use explicit indexing or kwarg-style format: "{}".format(str) is not

PS2, Line 1102:     if not (0 < cmd_idx <= history_length or -history_length <= cmd_idx
< 0):
> long line

Line 1107:       cmd_idx += history_length + 1
> Does it always have a semicolon? .rstrip(";") might be more robust and clea

Line 1156:         # The history file is not writable, disable readline.
> Can you mention that this method will perform transformations?

PS2, Line 1157: 
> what will 'line' contain?
One more line of comment is added.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc28e8ce07845343267224c3b9ccb71b29a524d2
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tianyi Wang <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tianyi Wang <>
Gerrit-HasComments: Yes

View raw message