impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <>
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:


Thanks Matthew. Please see patch set 3.
File tests/comparison/

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?
File tests/comparison/

PS2, Line 537:  is
> ?

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

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

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <>
Gerrit-Reviewer: David Knupp <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Harrison Sheinblatt <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-Reviewer: Taras Bobrovytsky <>
Gerrit-HasComments: Yes

View raw message