Tim Armstrong has posted comments on this change. Change subject: IMPALA-5184: build fe against both Hive 1 & 2 APIs ......................................................................
Patch Set 9: (8 comments) http://gerrit.cloudera.org:8080/#/c/5538/9/common/thrift/CMakeLists.txt File common/thrift/CMakeLists.txt: Line 189: # The thrift-generated java classes defined in TCLIService are also downloaded via maven. > Can you expand the first sentence to explain in more detail why the downloa Done. http://gerrit.cloudera.org:8080/#/c/5538/9/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 34: import org.apache.impala.authorization.User; > Why is there Impala stuff in here if this is a shim for the Hive Metastore? The problem is that the implementations of execGetThing() depend on declaring variables with types like TGetThingReq. To declare the variable, we need to import the class, and to import the class we need to name the package that the class is in. The last part is the problem. Line 44: * A wrapper around some of Hive's metastore API's to abstract away differences > Metastore Done Line 45: * between major versions of hive. This implements the shimmed methods for Hive 2. > Hive Done Line 47: public class MetaStoreShim { > Let's use "Metastore" and not "MetaStore". Impala uses the former consisten Done http://gerrit.cloudera.org:8080/#/c/5538/9/fe/src/compat-hive-2/java/org/apache/impala/compat/MetaStoreShim.java File fe/src/compat-hive-2/java/org/apache/impala/compat/MetaStoreShim.java: Line 44: * A wrapper around some of Hive's metastore API's to abstract away differences > Metastore Done Line 45: * between major versions of hive. This implements the shimmed methods for Hive 2. > Hive Done PS9, Line 100: p > This and the following functions look identical between the shim implementa The imports up the top of the file for TGetThingReq are different unfortunately. This is a workaround for the fact that there's no good way to do any kind of conditional important. -- 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: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
