Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-5184: build fe against both Hive 1 & 2 APIs ......................................................................
Patch Set 8: Code-Review+1 (6 comments) http://gerrit.cloudera.org:8080/#/c/5538/7/bin/impala-config.sh File bin/impala-config.sh: Line 377: # a complete Hive build. > I ran into the dreaded sticky config variable problem with some of these va Thanks for fixing it. http://gerrit.cloudera.org:8080/#/c/5538/7/common/thrift/.gitignore File common/thrift/.gitignore: Line 4: hive-2-api/TCLIService.thrift > The other one is checked in, so any changes to it shouldn't be ignored. Ok I wasn't aware of that. Thanks for clarifying. http://gerrit.cloudera.org:8080/#/c/5538/7/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: Line 189: # The thrift-generated java classes defined in TCLIService are also downloaded via maven. > TCLIService.thrift from Hive 2 adds additional RPC endpoints that we don't Makes sense. My only concern was that if the spec is updated, we may have to fix this logic again. However that looks less likely, so fine by me. http://gerrit.cloudera.org:8080/#/c/5538/8/fe/src/compat-hive-1/java/org/apache/hive/service/rpc/thrift/TGetTablesReq.java File fe/src/compat-hive-1/java/org/apache/hive/service/rpc/thrift/TGetTablesReq.java: Line 1: package org.apache.hive.service.rpc.thrift; Apache license in the newly added classes? http://gerrit.cloudera.org:8080/#/c/5538/8/fe/src/compat-hive-1/java/org/apache/impala/compat/MetaStoreShim.java File fe/src/compat-hive-1/java/org/apache/impala/compat/MetaStoreShim.java: Line 16: // under the License. nit: \n after license. http://gerrit.cloudera.org:8080/#/c/5538/7/fe/src/compat-hive-2/java/org/apache/impala/compat/MetaStoreCompatShim.java File fe/src/compat-hive-2/java/org/apache/impala/compat/MetaStoreCompatShim.java: Line 100 > The thrift classes moved between packages. The code is identical but the cl Ah, got it. -- To view, visit http://gerrit.cloudera.org:8080/5538 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifbc265281c04fe3136bc3c920dbac966742ce09a Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
