impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anonymous Coward (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3980: qgen: re-enable Hive as a target database
Date Fri, 23 Sep 2016 22:08:36 GMT
stakiar@cloudera.com has posted comments on this change.

Change subject: IMPALA-3980: qgen: re-enable Hive as a target database
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/4011/7/tests/comparison/cluster.py
File tests/comparison/cluster.py:

Line 100:     if result is None:
> I think it's a little confusing to check if default is None here.
Done


Line 185:     for file_name in ["hive-site.xml"]:
> I'm not sure that modifying the minicluster here is the right thing to do. 
I run the Hive tests against a local instance of Hive, so setting this to HIVE_CONF_DIR makes
development for me much. I think it will also be necessary to run the RQG against Hive in
general as Hive-on-RQG should be able to accept a custom hive-default.xml file.

I've tested this change against Impala (ran the data_generator, discpreancy_checker, and Leopard)
and it works fine. There were a few failures because now the the code uses fe/src/test/resources/hive-default.xml
which had some invalid xml clauses.

If you think this will cause problems for Impala, I can revert it. However, in that case I
would like to create a new Cluster object that can accept generic hive-default.xml files.


http://gerrit.cloudera.org:8080/#/c/4011/7/tests/comparison/data_generator.py
File tests/comparison/data_generator.py:

Line 92:     self.cluster = None
> How about adding a variable here called, for example, db_engine that can be
Done


Line 153:       with self.cluster.impala.cursor(db_name=self.db_name) as cursor:
> We don't need to compute stats if we are using hive?
Thats a good point, updated the patch to do this.


-- 
To view, visit http://gerrit.cloudera.org:8080/4011
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: stakiar@cloudera.com
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: stakiar@cloudera.com
Gerrit-HasComments: Yes

Mime
View raw message