impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Date Wed, 02 Nov 2016 19:43:29 GMT
Michael Brown has posted comments on this change.

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


Patch Set 3:

(4 comments)

Thanks Matthew. Please see patch set 3.

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

PS2, Line 527: updatable_columns
> it might be more helpful to use an existing concept, e.g. 'non_pk_cols' or 
I don't agree. I think a name like "non_pk_cols" can end up leading to confusion. What should
"non_pk_cols" do if the Table is, say, an Impala TEXTTABLE, or a Postgres table based off
a TEXTTABLE? Should it return all of the columns, because none are primary keys, or should
it return none of the columns, because that table doesn't have any primary keys? Someone then
has to go read the docstring to know. Meanwhile the property is needed for the query generator
to know what keys it may update.

So I think updatable_columns is a more accurate name.

Can you comment, or mark Done if this is OK?


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

PS2, Line 537:  is
> ?
Done


PS2, Line 695: EATE TABLE db.table (
> can you comment if this is the same format printed for 1 PK col as well?
Done


http://gerrit.cloudera.org:8080/#/c/4873/2/tests/conftest.py
File tests/conftest.py:

Line 456:   generator, stress test, etc.
> but why is this necessary?
Done


-- 
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: 3
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

Mime
View raw message