hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sergey Shelukhin" <ser...@hortonworks.com>
Subject Re: Review Request 40867: HIVE-11527 - bypass HiveServer2 thrift interface for query results
Date Thu, 03 Dec 2015 21:40:10 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40867/#review108889
-----------------------------------------------------------



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 390)
<https://reviews.apache.org/r/40867/#comment168346>

    some existing configuration is probably needed (and better)



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 401)
<https://reviews.apache.org/r/40867/#comment168348>

    I am not sure how the original rowset works; this may not be good for large result sets.
Should be ok as long as we have parity with the original, but original has fetchSize parameter
which I assume could be doing some batching. That might be useful to have



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 402)
<https://reviews.apache.org/r/40867/#comment168351>

    also, I wonder if it makes sense to add rows to rowset immediately after reading. Storing
all rows and then adding them all to rowset stores all rows twice in memory.



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 455)
<https://reviews.apache.org/r/40867/#comment168349>

    nit: why object?



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 465)
<https://reviews.apache.org/r/40867/#comment168350>

    I wonder why we need to have thrift serialization? One of the goals is to avoid it. Perhaps
it can be done in a follow-up JIRA



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 485)
<https://reviews.apache.org/r/40867/#comment168352>

    nit: useless check, schema is already used above to get descriptors



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 491)
<https://reviews.apache.org/r/40867/#comment168353>

    nit: get(pos) can be done once



jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java (line 506)
<https://reviews.apache.org/r/40867/#comment168354>

    again please use existing conf object


- Sergey Shelukhin


On Dec. 2, 2015, 12:52 p.m., Takanobu Asanuma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40867/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2015, 12:52 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> This is a WIP patch for HIVE-11527
> 
> * I added a new configuration whose name is hive.server2.webhdfs.bypass.enabled. The
default is false. When this value is true, clients use the bypass.
> 
> * I still have not considered security such as Kerberos and SSL at present.
> 
> * I have not implement Statement#setFetchSize for bypass yet.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java db942b0 
>   jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 245c6a3 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 180f99e8 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 8fafd61 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FetchTask.java 1634143 
>   service/if/TCLIService.thrift baf583f 
>   service/src/gen/thrift/gen-cpp/TCLIService_types.h b078c99 
>   service/src/gen/thrift/gen-cpp/TCLIService_types.cpp b852379 
>   service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TExecuteStatementResp.java
0b9aa0f 
>   service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TProtocolVersion.java
c936ada 
>   service/src/gen/thrift/gen-py/TCLIService/ttypes.py ef5f5f5 
>   service/src/gen/thrift/gen-py/hive_service/ThriftHive-remote e167d5b 
>   service/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb f004ec4 
>   service/src/java/org/apache/hive/service/cli/CLIService.java adc9809 
>   service/src/java/org/apache/hive/service/cli/operation/Operation.java 25cefc2 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java b0bd351

>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 1331a99 
>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 4f4e92d 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java a14908b 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 8434965 
> 
> Diff: https://reviews.apache.org/r/40867/diff/
> 
> 
> Testing
> -------
> 
> I have tested few simple queries and they worked well. But I think there are some problems
for some queries. I'm going to test more queries and fix bugs. I'm also going to add unit
tests.
> 
> 
> Thanks,
> 
> Takanobu Asanuma
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message