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