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 <mi...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com>
Gerrit-Reviewer: Harrison Sheinblatt <h...@hotmail.com>
Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to