[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. IMPALA-4352: test infra: store Impala/Kudu primary keys in object model Test infrastructure, including the random query generator and the data migrator, needs to know the primary keys of Impala/Kudu tables. This test infrastructure keeps Python object models of the tables and columns. This patch adds the ability to read from source Impala/Kudu tables via SHOW CREATE TABLE and store primary keys as proper attributes. The patch also adds tests that ensure the test infrastructure is always able to read and store the primary keys. This helps find breakages sooner rather than later. For example, if a regression to "SHOW CREATE TABLE" or the test infrastructure makes us no longer able to parse primary keys, GVO or other CI will find the breakage faster than running the query generator. I also fixed some flake8 issues in files I touched. There were several files that had a lot of white space warnings, and I wanted to keep the patch from getting too large. Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 Reviewed-on: http://gerrit.cloudera.org:8080/4873 Reviewed-by: Michael Brown Reviewed-by: Taras Bobrovytsky Tested-by: Internal Jenkins --- M tests/comparison/common.py M tests/comparison/db_connection.py M tests/conftest.py M tests/metadata/test_show_create_table.py 4 files changed, 175 insertions(+), 29 deletions(-) Approvals: Michael Brown: Looks good to me, but someone else must approve Taras Bobrovytsky: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 Gerrit-PatchSet: 8 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: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 7: Verified+1 -- 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: 7 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: Internal Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 7: Thanks for the review, Taras. This *does* need GVO, which I'll make sure to do. -- 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: 7 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: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 7: Code-Review+2 -- 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: 7 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: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 7: Code-Review+1 (1 comment) Thanks for the review. I made the rename and ran the tests locally. http://gerrit.cloudera.org:8080/#/c/4873/5/tests/comparison/db_connection.py File tests/comparison/db_connection.py: PS5, Line 532: _fet > Maybe it would be better to call this something other than get? Maybe 'load 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: 7 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Hello Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4873 to look at the new patch set (#7). Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. IMPALA-4352: test infra: store Impala/Kudu primary keys in object model Test infrastructure, including the random query generator and the data migrator, needs to know the primary keys of Impala/Kudu tables. This test infrastructure keeps Python object models of the tables and columns. This patch adds the ability to read from source Impala/Kudu tables via SHOW CREATE TABLE and store primary keys as proper attributes. The patch also adds tests that ensure the test infrastructure is always able to read and store the primary keys. This helps find breakages sooner rather than later. For example, if a regression to "SHOW CREATE TABLE" or the test infrastructure makes us no longer able to parse primary keys, GVO or other CI will find the breakage faster than running the query generator. I also fixed some flake8 issues in files I touched. There were several files that had a lot of white space warnings, and I wanted to keep the patch from getting too large. Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 --- M tests/comparison/common.py M tests/comparison/db_connection.py M tests/conftest.py M tests/metadata/test_show_create_table.py 4 files changed, 175 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/4873/7 -- To view, visit http://gerrit.cloudera.org:8080/4873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 Gerrit-PatchSet: 7 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 6: Code-Review+1 rebase -- 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: 6 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: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/4873/5/tests/comparison/db_connection.py File tests/comparison/db_connection.py: PS5, Line 532: _get Maybe it would be better to call this something other than get? Maybe 'load' or 'fetch'. To me get sounds like it's returning some value in a local varaible. -- 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: 5 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 5: Code-Review+1 rebase -- 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: 5 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: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 4: Code-Review+1 -- 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: 4 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: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/4873/2/tests/comparison/common.py File tests/comparison/common.py: PS2, Line 527: updatable_columns > I don't agree. I think a name like "non_pk_cols" can end up leading to conf Sure, I don't feel strongly, but I don't think of it being weird for other table types -- they really have no notion of a primary key, so all cols are not PK cols. It's like saying we can't ask if a col is decimal or not just because a particular storage format doesn't support decimal. Further, it is common to reason about the abstraction of a "Table" as something having PKs or not, whereas 'updatable' isn't a property that is common on this abstraction. Anyway, that's just what I was thinking. I'm fine either way. -- 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: 2 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 4: Patch set 4 adds Table object properties for the names of the columns in addition to what we had before, which were full Column objects. I find myself checking the name, so this would be convenient to have. It also makes 1 of the tests a bit cleaner. -- 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: 4 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: No
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has uploaded a new patch set (#4). Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. IMPALA-4352: test infra: store Impala/Kudu primary keys in object model Test infrastructure, including the random query generator and the data migrator, needs to know the primary keys of Impala/Kudu tables. This test infrastructure keeps Python object models of the tables and columns. This patch adds the ability to read from source Impala/Kudu tables via SHOW CREATE TABLE and store primary keys as proper attributes. The patch also adds tests that ensure the test infrastructure is always able to read and store the primary keys. This helps find breakages sooner rather than later. For example, if a regression to "SHOW CREATE TABLE" or the test infrastructure makes us no longer able to parse primary keys, GVO or other CI will find the breakage faster than running the query generator. I also fixed some flake8 issues in files I touched. There were several files that had a lot of white space warnings, and I wanted to keep the patch from getting too large. Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 --- M tests/comparison/common.py M tests/comparison/db_connection.py M tests/conftest.py M tests/metadata/test_show_create_table.py 4 files changed, 175 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/4873/4 -- To view, visit http://gerrit.cloudera.org:8080/4873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 Gerrit-PatchSet: 4 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
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 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has uploaded a new patch set (#3). Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. IMPALA-4352: test infra: store Impala/Kudu primary keys in object model Test infrastructure, including the random query generator and the data migrator, needs to know the primary keys of Impala/Kudu tables. This test infrastructure keeps Python object models of the tables and columns. This patch adds the ability to read from source Impala/Kudu tables via SHOW CREATE TABLE and store primary keys as proper attributes. The patch also adds tests that ensure the test infrastructure is always able to read and store the primary keys. This helps find breakages sooner rather than later. For example, if a regression to "SHOW CREATE TABLE" or the test infrastructure makes us no longer able to parse primary keys, GVO or other CI will find the breakage faster than running the query generator. I also fixed some flake8 issues in files I touched. There were several files that had a lot of white space warnings, and I wanted to keep the patch from getting too large. Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 --- M tests/comparison/common.py M tests/comparison/db_connection.py M tests/conftest.py M tests/metadata/test_show_create_table.py 4 files changed, 166 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/4873/3 -- To view, visit http://gerrit.cloudera.org:8080/4873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 2: (4 comments) 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 something like that http://gerrit.cloudera.org:8080/#/c/4873/2/tests/comparison/db_connection.py File tests/comparison/db_connection.py: PS2, Line 537: () ? PS2, Line 695: PRIMARY KEY (pk1, pk2) can you comment if this is the same format printed for 1 PK col as well? 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? -- 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: 2 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 2: (1 comment) Thanks for the comment, Taras. Please set patch set 2. http://gerrit.cloudera.org:8080/#/c/4873/1/tests/comparison/db_connection.py File tests/comparison/db_connection.py: Line 407: table.add_col(col) > col.is_primary_key = col_name in primary_key_names 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: 2 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has uploaded a new patch set (#2). Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. IMPALA-4352: test infra: store Impala/Kudu primary keys in object model Test infrastructure, including the random query generator and the data migrator, needs to know the primary keys of Impala/Kudu tables. This test infrastructure keeps Python object models of the tables and columns. This patch adds the ability to read from source Impala/Kudu tables via SHOW CREATE TABLE and store primary keys as proper attributes. The patch also adds tests that ensure the test infrastructure is always able to read and store the primary keys. This helps find breakages sooner rather than later. For example, if a regression to "SHOW CREATE TABLE" or the test infrastructure makes us no longer able to parse primary keys, GVO or other CI will find the breakage faster than running the query generator. I also fixed some flake8 issues in files I touched. There were several files that had a lot of white space warnings, and I wanted to keep the patch from getting too large. Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 --- M tests/comparison/common.py M tests/comparison/db_connection.py M tests/conftest.py M tests/metadata/test_show_create_table.py 4 files changed, 151 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/4873/2 -- To view, visit http://gerrit.cloudera.org:8080/4873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 Gerrit-PatchSet: 2 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Taras Bobrovytsky has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/4873/1/tests/comparison/db_connection.py File tests/comparison/db_connection.py: Line 407: col.is_primary_key = True col.is_primary_key = col_name in primary_key_names -- 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 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
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: (1 comment) 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 It turns out the ordering is based off the declaration ordering. See IMPALA-4402 -- 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 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
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 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
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
David Knupp has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 1: (2 comments) 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 only returns an empty tuple. 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_HOME}/tests called "infra", as a place to keep (an hopefully add) tests of the infrastructure itself. What do you think of that idea? -- 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 Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Harrison Sheinblatt Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Harrison Sheinblatt has posted comments on this change. Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. Patch Set 1: (1 comment) 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 independent of declaration order of columns in the create table statement. I think it would be better to store the primary keys as a separate list like _cols to maintain that order information in the Table class. -- 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 Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Harrison Sheinblatt Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Taras Bobrovytsky Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4352: test infra: store Impala/Kudu primary keys in object model
Michael Brown has uploaded a new change for review. http://gerrit.cloudera.org:8080/4873 Change subject: IMPALA-4352: test infra: store Impala/Kudu primary keys in object model .. IMPALA-4352: test infra: store Impala/Kudu primary keys in object model Test infrastructure, including the random query generator and the data migrator, needs to know the primary keys of Impala/Kudu tables. This test infrastructure keeps Python object models of the tables and columns. This patch adds the ability to read from source Impala/Kudu tables via SHOW CREATE TABLE and store primary keys as proper attributes. The patch also adds tests that ensure the test infrastructure is always able to read and store the primary keys. This helps find breakages sooner rather than later. For example, if a regression to "SHOW CREATE TABLE" or the test infrastructure makes us no longer able to parse primary keys, GVO or other CI will find the breakage faster than running the query generator. I also fixed some flake8 issues in files I touched. There were several files that had a lot of white space warnings, and I wanted to keep the patch from getting too large. Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 --- M tests/comparison/common.py M tests/comparison/db_connection.py M tests/conftest.py M tests/metadata/test_show_create_table.py 4 files changed, 151 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/4873/1 -- To view, visit http://gerrit.cloudera.org:8080/4873 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib654b6cd0e8c2a172ffb7330497be4d4a751e6e5 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Michael Brown