Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201610311608.u9VG8XL5023047@ip-10-146-233-104.ec2.internal>
Date: Mon, 31 Oct 2016 16:08:33 +0000
From: "Michael Brown (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Matthew Jacobs <mj@cloudera.com>,
        Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>,
        Taras Bobrovytsky <tbobrovytsky@cloudera.com>,
        Harrison Sheinblatt <hs7@hotmail.com>,
        David Knupp <dknupp@cloudera.com>
Reply-To: mikeb@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-4352=3A_test_infra=3A_store_Impala/Kudu_primary_keys_in_object_model=0A?=
In-Reply-To: <gerrit.1477603050000.Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5@gerrit.cloudera.org>
References: <gerrit.1477603050000.Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5@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: Mon, 31 Oct 2016 16:08:40 -0000

Michael Brown has posted comments on this change.

Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
......................................................................


Patch Set 1:

(3 comments)

Thanks for the reviews.

http://gerrit.cloudera.org:8080/#/c/4873/1/tests/comparison/common.py
File tests/comparison/common.py:

Line 524:     return tuple(col for col in self._cols if col.is_primary_key)
> In general I believe primary keys to be ordered, and that the order is inde
I'm following up offline with Dimitris about this. How I fix this depends on the answer.


http://gerrit.cloudera.org:8080/#/c/4873/1/tests/comparison/db_connection.py
File tests/comparison/db_connection.py:

PS1, Line 538: return ()
> What am I missing here? This doesn't do what the docstring claims -- it onl
This is the base class method. We're only implementing the parsing for Impala tables at the moment. Note the override further below. Also, "or an empty tuple if there are no primary keys".


http://gerrit.cloudera.org:8080/#/c/4873/1/tests/metadata/test_show_create_table.py
File tests/metadata/test_show_create_table.py:

PS1, Line 241: TestInfraCompat
> I have wondered whether we should create a new directory under ${IMPALA_HOM
The tests in this case are very close to the edge of test infrastructure meeting with product. We need these tests to run continuously to catch changes in SHOW CREATE TABLE output. This differs from the tests in tests/comparison/tests, which don't really depend on changes in the product. So, I put these tests into a location that would hit GVO and CI builds.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Harrison Sheinblatt <hs7@hotmail.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes