has posted comments on this change.

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

Patch Set 8:

File tests/comparison/

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

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.
File tests/comparison/

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

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifb1199b50a5b65c21de7876fb70cc03bda1a9b46
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Reviewer: David Knupp <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Taras Bobrovytsky <>
Gerrit-HasComments: Yes

Reply via email to