stak...@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: stak...@cloudera.com
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: stak...@cloudera.com
Gerrit-HasComments: Yes

Reply via email to