yx91...@126.com has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18549 )

Change subject: IMPALA-1682: Support printing the output of a query (rows) 
vertically.
......................................................................


Patch Set 7:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/18549/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/18549/5//COMMIT_MSG@7
PS5, Line 7: print
> nit: printing
Done


http://gerrit.cloudera.org:8080/#/c/18549/5//COMMIT_MSG@9
PS5, Line 9: impa
> nit: impala-shell will ...
Done


http://gerrit.cloudera.org:8080/#/c/18549/5/docs/topics/impala_shell_options.xml
File docs/topics/impala_shell_options.xml:

http://gerrit.cloudera.org:8080/#/c/18549/5/docs/topics/impala_shell_options.xml@193
PS5, Line 193:                 <p> -E or </p>
> Thanks for adding the document!
Done


http://gerrit.cloudera.org:8080/#/c/18549/5/docs/topics/impala_shell_options.xml@203
PS5, Line 203: ode
> We are going to release 4.1 now. Please use 4.2.
Done


http://gerrit.cloudera.org:8080/#/c/18549/2/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/18549/2/shell/impala_shell.py@170
PS2, Line 170: :
> flake8: E203 whitespace before ':'
Done


http://gerrit.cloudera.org:8080/#/c/18549/2/shell/shell_output.py
File shell/shell_output.py:

http://gerrit.cloudera.org:8080/#/c/18549/2/shell/shell_output.py@110
PS2, Line 110:
> flake8: E501 line too long (127 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/18549/5/shell/shell_output.py
File shell/shell_output.py:

http://gerrit.cloudera.org:8080/#/c/18549/5/shell/shell_output.py@98
PS5, Line 98:
> nit: I think "column_name_max_len" is better
Done


http://gerrit.cloudera.org:8080/#/c/18549/5/shell/shell_output.py@108
PS5, Line 108:  val in row]
> nit: We can use the utf8_encode_if_needed() method instead:
It seems that I should use "from shell.impala_client import 
utf8_encode_if_needed" to import, cause shell_output.py depends on 
impala_client.py, is that OK?


http://gerrit.cloudera.org:8080/#/c/18549/5/shell/shell_output.py@110
PS5, Line 110: ow ***********************************
> I'm curious how does other systems, e.g. MySQL, determine the wide of this.
the width in mysql is also fixed: 
https://github.com/mysql/mysql-server/blob/mysql-5.5.24/client/mysql.cc#L3577.
calcuate the size base on terminal width is realizable,  but will bring more 
difficulties to tests.
I prefer to follow the established rules, do you agree?


http://gerrit.cloudera.org:8080/#/c/18549/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/18549/2/tests/shell/test_shell_commandline.py@309
PS2, Line 309:
> flake8: E501 line too long (114 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/18549/5/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:

http://gerrit.cloudera.org:8080/#/c/18549/5/tests/shell/test_shell_commandline.py@295
PS5, Line 295: select 1 as
> Can we change this to 'select 1 first, 1 second, 1 third' so we can test th
Done


http://gerrit.cloudera.org:8080/#/c/18549/2/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/18549/2/tests/shell/test_shell_interactive.py@259
PS2, Line 259:
> flake8: E501 line too long (128 > 90 characters)
Done


http://gerrit.cloudera.org:8080/#/c/18549/5/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/18549/5/tests/shell/test_shell_interactive.py@262
PS5, Line 262: "n_name: "
> nit: it'd be nice to verify the alignment, i.e. use "     n_name: " instead
Done



--
To view, visit http://gerrit.cloudera.org:8080/18549
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5cee48d5a239d6b7c0f51331275524a25130fadf
Gerrit-Change-Number: 18549
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <yx91...@126.com>
Gerrit-Reviewer: Anonymous Coward <yx91...@126.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Comment-Date: Thu, 26 May 2022 06:29:44 +0000
Gerrit-HasComments: Yes

Reply via email to