> On 1月 14, 2016, 5:36 p.m., Prasad Mujumdar wrote: > > My apologies for jumping in late in the review > > > > I am curious about why you chose HiveServer2 interface to implement this. I > > guess one can work it around by 'INSERT OVERWRITE DIRECTORY .. SELECT ..' > > and so one of the options could be to solve this in the query processing > > (eg rewrite the query during parsing etc). The problem for HS2 solution is > > that it's not going to be accessible to high level clients like > > JDBC/ODBC/Beeline. One has to write a custom HS2 client driver in order to > > pass the new parameters. > > > > Regarding this patch, one comment below.
Thank you for your review and comments, Prasad! > I am curious about why you chose HiveServer2 interface to implement this. When some JDBC clients download large data from HiveServer2, HiveServer2 will be a bottle neck. This bypass is for between JDBC clients and HDFS so that JDBC clients can download data from HDFS directly. > I guess one can work it around by 'INSERT OVERWRITE DIRECTORY .. SELECT ..' > and so one of the options could be to solve this in the query processing (eg > rewrite the query during parsing etc). > The problem for HS2 solution is that it's not going to be accessible to high > level clients like JDBC/ODBC/Beeline. What do you mean by that? I rather think this bypass would not work 'INSERT OVERWRITE DIRECTORY .. SELECT ..'. Because it does not return results to JDBC clients. > On 1月 14, 2016, 5:36 p.m., Prasad Mujumdar wrote: > > service/if/TCLIService.thrift, line 272 > > <https://reviews.apache.org/r/40867/diff/4/?file=1187218#file1187218line272> > > > > Does it need to be a required parameter ? Adding a new required > > parameter in the middle will make it break the backward compatibility for > > existing clients. Yes, it is necessary to be a required parameter since this patch had TypeDescriptor created by the typename in ColumnDescriptor. (Please see last Sergey's review.) Next patch fixes the order of the parameters. - Takanobu ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40867/#review114504 ----------------------------------------------------------- On 1月 8, 2016, 7:18 a.m., Takanobu Asanuma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40867/ > ----------------------------------------------------------- > > (Updated 1月 8, 2016, 7:18 a.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 479fa46 > jdbc/src/java/org/apache/hive/jdbc/HiveQueryResultSet.java 245c6a3 > jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java 3f9024a > ql/src/java/org/apache/hadoop/hive/ql/Driver.java 29e6315 > 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/TColumnDesc.java > b01fadb > > 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-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/ColumnDescriptor.java f0bbf14 > service/src/java/org/apache/hive/service/cli/operation/Operation.java > d2b3f9c > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > 92135cd > service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java > 2eaab4a > service/src/java/org/apache/hive/service/cli/session/HiveSession.java > 4f4e92d > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > 50c912e > 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 > >
