Adam Tamas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16305 )

Change subject: WIP IMPALA-10012: ds_hll_sketch() results ascii codec decoding 
error fix
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16305/3/shell/impala_client.py
File shell/impala_client.py:

http://gerrit.cloudera.org:8080/#/c/16305/3/shell/impala_client.py@1100
PS3, Line 1100:   try:
> It could be potentially faster if the exception was caught per row instead
Done


http://gerrit.cloudera.org:8080/#/c/16305/3/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/16305/3/shell/impala_shell.py@1191
PS3, Line 1191:         # retrieve the error log
> Where does the exception come from?
Here, it can come from 2 different sources depending on which formatter we are 
using (DelimitedOutputFormatter or PrettyOutputFormatter from the 
shell_output.py) Moved the check there.


This is only true, if we are using the gen-py generated by thrift 0.9.3, if it 
is 0.11.0 the error is coming from the thrift autogenerated files. (Here is no 
solution for thrift 0.11.0 yet)


http://gerrit.cloudera.org:8080/#/c/16305/3/shell/impala_shell.py@1197
PS3, Line 1197: if
> What is "i", a character? I would prefer a name like "ch" in that case, as
THANKS!
This was a problem I was looking a solution for.
(The Dwarves delved too greedily and too deep)


http://gerrit.cloudera.org:8080/#/c/16305/3/tests/shell/test_shell_interactive.py
File tests/shell/test_shell_interactive.py:

http://gerrit.cloudera.org:8080/#/c/16305/3/tests/shell/test_shell_interactive.py@289
PS3, Line 289:
> I would prefer to test against a predefined string, e.g. one that contains
While it is theoretically done, since the tests use thrift 0.11.0 this test 
will fail for now.


http://gerrit.cloudera.org:8080/#/c/16305/3/tests/shell/test_shell_interactive.py@292
PS3, Line 292:     child_proc = spawn_shell(shell_cmd)
> This path tests the shell when it uses the pretty printer - can you also te
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5cfb907871ca83e5f04a39ca9d7a8e138d711a8
Gerrit-Change-Number: 16305
Gerrit-PatchSet: 4
Gerrit-Owner: Adam Tamas <[email protected]>
Gerrit-Reviewer: Adam Tamas <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Thu, 13 Aug 2020 10:05:42 +0000
Gerrit-HasComments: Yes

Reply via email to