Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/18549 )
Change subject: IMPALA-1682: Support print the output of a query (rows)
vertically.
......................................................................
Patch Set 5:
(9 comments)
Thanks for your contribution! The solution looks good to me. There are some
test failures. Could you fix them?
shell/test_shell_commandline.py:307: in test_output_format
assert len(result_rows) == 4
E assert 1 == 4
E + where 1 = len(['1\t1\t1'])
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16542/
You can run the test by
impala-py.test
tests/shell/test_shell_commandline.py::TestImpalaShell::test_output_format
Remember to recompiling impala-shell if you change any related codes.
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
http://gerrit.cloudera.org:8080/#/c/18549/5//COMMIT_MSG@9
PS5, Line 9: will
nit: impala-shell will ...
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!
http://gerrit.cloudera.org:8080/#/c/18549/5/docs/topics/impala_shell_options.xml@203
PS5, Line 203: 4.1
We are going to release 4.1 now. Please use 4.2.
Please also mention "No effects if -B is used"
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: column_name_len_max
nit: I think "column_name_max_len" is better
http://gerrit.cloudera.org:8080/#/c/18549/5/shell/shell_output.py@108
PS5, Line 108: val.encode('utf-8', 'replace') if isinstance(val, unicode) else
val
nit: We can use the utf8_encode_if_needed() method instead:
https://github.com/apache/impala/blob/882b6daf24a5be32706a2c1b8c069254d2f14033/shell/impala_client.py#L93
http://gerrit.cloudera.org:8080/#/c/18549/5/shell/shell_output.py@110
PS5, Line 110: **************************************
I'm curious how does other systems, e.g. MySQL, determine the wide of this. Can
we get the terminal wide and calculate the size based on it?
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,1,1
Can we change this to 'select 1 first, 1 second, 1 third' so we can test the
column names as well?
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.
--
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: 5
Gerrit-Owner: Anonymous Coward <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Tue, 24 May 2022 09:47:01 +0000
Gerrit-HasComments: Yes
