Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12884 )

Change subject: IMPALA-7290: part 2: Add HS2 support to Impala shell
......................................................................


Patch Set 16:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/12884/16/be/src/service/client-request-state.h
File be/src/service/client-request-state.h:

http://gerrit.cloudera.org:8080/#/c/12884/16/be/src/service/client-request-state.h@167
PS16, Line 167: 'request_state'
> I assume you mean 'this request'?
Done


http://gerrit.cloudera.org:8080/#/c/12884/16/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

http://gerrit.cloudera.org:8080/#/c/12884/16/common/thrift/ImpalaService.thrift@453
PS16, Line 453: operation_handle
> Unfortunately, our casing in this file is inconsistent, but I think you pro
Done


http://gerrit.cloudera.org:8080/#/c/12884/16/common/thrift/ImpalaService.thrift@545
PS16, Line 545:   // Same as HS2 CloseOperation but can return additional 
information.
> There's also the issue of running non-Impala clients against Impala, eg. th
Weirdly the number of modified rows was returned in the operation handle: 
https://github.com/apache/impala/blob/master/common/thrift/hive-1-api/TCLIService.thrift#L518

It looks like this was fixed in upstream hive: 
https://github.com/apache/hive/commit/da13a13797c7d9926f9a2332ee7b33c7b5f08a1e 
although it still doesn't have a # errors count.

Interestingly there's a bunch of progress information returned in that API - it 
seems like we could possibly port over to using that instead of the 
GetExecSummary() reporting (as part of a future change!) if we wanted to 
provide better beeline support.


http://gerrit.cloudera.org:8080/#/c/12884/16/shell/impala_shell.py
File shell/impala_shell.py:

http://gerrit.cloudera.org:8080/#/c/12884/16/shell/impala_shell.py@842
PS16, Line 842: # raise
> ?
Done


http://gerrit.cloudera.org:8080/#/c/12884/16/tests/hs2/test_hs2.py
File tests/hs2/test_hs2.py:

http://gerrit.cloudera.org:8080/#/c/12884/16/tests/hs2/test_hs2.py@100
PS16, Line 100: # Removed options are returned.
> Isn't this still testing that they're not returned?
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d5cc83d545aacc659523f29b1d6feed672e2a12
Gerrit-Change-Number: 12884
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Mon, 20 May 2019 21:51:49 +0000
Gerrit-HasComments: Yes

Reply via email to