----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35792/#review102547 -----------------------------------------------------------
Besides the comments in the code, I have the following questions: 1. Do we have to support multiple compressors and otherwise the functionality is incomplete? Or, can we support one and later enhance if demain arises? 2. As to client telling the server its capability regarding supported compressor, what's the practice in the traditional databases? Is JSON string the typical practice. I feel that JSON is a little unwieldy. service/if/TCLIService.thrift (line 406) <https://reviews.apache.org/r/35792/#comment160251> What's the use of type and size? Are these not available by looking at result set schema? service/src/java/org/apache/hive/service/cli/compression/ColumnCompressorService.java (line 34) <https://reviews.apache.org/r/35792/#comment160249> It seems that calling this as a serive is a little confusing. Maybe we can just call it "factory"? service/src/java/org/apache/hive/service/cli/compression/EncodedColumnBasedSet.java (line 92) <https://reviews.apache.org/r/35792/#comment160246> If hiveConf is mandatory, should we enforce it by requiring it in the constructor rather than a set method? service/src/java/org/apache/hive/service/cli/compression/EncodedColumnBasedSet.java (line 110) <https://reviews.apache.org/r/35792/#comment160248> Can we do this once per user session instead of per result set? - Xuefu Zhang On Aug. 17, 2015, 10:37 p.m., Rohit Dholakia wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35792/ > ----------------------------------------------------------- > > (Updated Aug. 17, 2015, 10:37 p.m.) > > > Review request for hive, Vaibhav Gumashta, Xiaojian Wang, Xiao Meng, and > Xuefu Zhang. > > > Repository: hive-git > > > Description > ------- > > This patch enables ResultSet compression for Hive using external plugins. The > patch proposes a plugin architecture that enables using external plugins to > compress ResultSets on-the-fly. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 730f5be > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java bb2b695 > service/if/TCLIService.thrift baf583f > service/src/gen/thrift/gen-cpp/TCLIService.h 29a9f4a > service/src/gen/thrift/gen-cpp/TCLIService_types.h 4536b41 > service/src/gen/thrift/gen-cpp/TCLIService_types.cpp 742cfdc > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TEnColumn.java > PRE-CREATION > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TExecuteStatementReq.java > feaed34 > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TGetTablesReq.java > 805e69f > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TOpenSessionReq.java > 657f868 > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TOpenSessionResp.java > 48f4b45 > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TProtocolVersion.java > 6e714c6 > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TRowSet.java > cc1a148 > > service/src/gen/thrift/gen-javabean/org/apache/hive/service/cli/thrift/TStatus.java > 1cd7980 > service/src/gen/thrift/gen-py/TCLIService/ttypes.py efee8ef > service/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb bfb2b69 > service/src/java/org/apache/hive/service/cli/Column.java 2e21f18 > service/src/java/org/apache/hive/service/cli/ColumnBasedSet.java 47a582e > service/src/java/org/apache/hive/service/cli/RowSetFactory.java e8f68ea > > service/src/java/org/apache/hive/service/cli/compression/ColumnCompressor.java > PRE-CREATION > > service/src/java/org/apache/hive/service/cli/compression/ColumnCompressorService.java > PRE-CREATION > > service/src/java/org/apache/hive/service/cli/compression/EncodedColumnBasedSet.java > PRE-CREATION > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > 67bc778 > > service/src/test/org/apache/hive/service/cli/compression/SnappyIntCompressor.java > PRE-CREATION > > service/src/test/org/apache/hive/service/cli/compression/TestEncodedColumnBasedSet.java > PRE-CREATION > > service/src/test/resources/META-INF/services/org.apache.hive.service.cli.compression.ColumnCompressor > PRE-CREATION > > Diff: https://reviews.apache.org/r/35792/diff/ > > > Testing > ------- > > Testing has been done using a docker container-based query submitter that has > an integer decompressor as part of it. Using the integer compressor (also > provided) and the decompressor, the end-to-end functionality can be observed. > > > File Attachments > ---------------- > > Patch file > > https://reviews.apache.org/media/uploaded/files/2015/06/23/16aa08f8-2393-460a-83ef-72464fc537db__HIVE-10438.patch > > > Thanks, > > Rohit Dholakia > >
