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 <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Harrison Sheinblatt <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
