impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5184: build fe against both Hive 1 & 2 APIs
Date Mon, 17 Apr 2017 23:41:50 GMT
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 <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message