Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201609211830.u8LIUa3a018155@ip-10-146-233-104.ec2.internal>
Date: Wed, 21 Sep 2016 18:30:36 +0000
From: "Michael Brown (Code Review)" <gerrit@cloudera.org>
To: stakiar@cloudera.com, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Taras Bobrovytsky <tbobrovytsky@cloudera.com>,
        David Knupp <dknupp@cloudera.com>
Reply-To: mikeb@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4101=3A_qgen=3A_Hive_join_predicates_should_only_contains_equality_functions=0A?=
In-Reply-To: <gerrit.1473901689000.Ibe8832a03cfa0d7ecc293ec6db6db2bcb34ab459@gerrit.cloudera.org>
References: <gerrit.1473901689000.Ibe8832a03cfa0d7ecc293ec6db6db2bcb34ab459@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Wed, 21 Sep 2016 18:30:42 -0000

Michael Brown has posted comments on this change.

Change subject: IMPALA-4101: qgen: Hive join predicates should only contains equality functions
......................................................................


Patch Set 3:

(3 comments)

Thanks for adding a test!

http://gerrit.cloudera.org:8080/#/c/4419/3/tests/comparison/query_generator.py
File tests/comparison/query_generator.py:

Line 1364:     '''
Add documentation about the signatures parameter.


http://gerrit.cloudera.org:8080/#/c/4419/3/tests/comparison/query_profile.py
File tests/comparison/query_profile.py:

PS3, Line 661:       if signature.func in (Equals, And):
             :         allowed_join_signatures.append(signature)
             :       elif len(signature.args) == 1:
Maybe use an or expression for these two conditions and remove the elif?


http://gerrit.cloudera.org:8080/#/c/4419/3/tests/comparison/tests/hive/test_create_join_clause.py
File tests/comparison/tests/hive/test_create_join_clause.py:

PS3, Line 31:   class MockQueryProfile(HiveProfile):
1. I prefer "Fake" instead of "Mock". I prefer the use of "mock" only when using actual mock libraries. Is this reasonable?

2. Use Hive in the name, to explain it's a fake HiveProfile.


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

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