[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. IMPALA-3491: Use unique database fixture in test_ddl.py. Adds new parametrization to the unique database fixture: - num_dbs: allows creating multiple unique databases at once; the 2nd, 3rd, etc. datbase name is generated by appending "2", "3", etc., to the first database name - sync_ddl: allows creating the dabatases(s) with sync_ddl which is needed by most tests in test_ddl.py Testing: I ran debug/core and debug/exhaustive on HDFS and core/debug on S3. Also ran the test locally in a loop on exhaustive. Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Reviewed-on: http://gerrit.cloudera.org:8080/4155 Reviewed-by: Alex Behm Tested-by: Internal Jenkins --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/create-database.test M testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test M testdata/workloads/functional-query/queries/QueryTest/create-table.test M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test M tests/common/parametrize.py M tests/conftest.py M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py M tests/metadata/test_hms_integration.py 13 files changed, 566 insertions(+), 740 deletions(-) Approvals: Internal Jenkins: Verified Alex Behm: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Internal Jenkins has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 6: Code-Review+2 Thanks Michael! -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 5: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Hello Michael Brown, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4155 to look at the new patch set (#5). Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. IMPALA-3491: Use unique database fixture in test_ddl.py. Adds new parametrization to the unique database fixture: - num_dbs: allows creating multiple unique databases at once; the 2nd, 3rd, etc. datbase name is generated by appending "2", "3", etc., to the first database name - sync_ddl: allows creating the dabatases(s) with sync_ddl which is needed by most tests in test_ddl.py Testing: I ran debug/core and debug/exhaustive on HDFS and core/debug on S3. Also ran the test locally in a loop on exhaustive. Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/create-database.test M testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test M testdata/workloads/functional-query/queries/QueryTest/create-table.test M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test M tests/common/parametrize.py M tests/conftest.py M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py M tests/metadata/test_hms_integration.py 13 files changed, 566 insertions(+), 740 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4155/5 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4155/4/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS4, Line 339: Note that we cannot run this test in parallel : # with multiple test vectors because the test modifies and checks a single : # impalad metric. > Let's talk in person about this test tomorrow. As discussed: * cleaned up the sync_ddl parametrization * moved the lib cache tests into their own class * marked one of them as serial with a comment why -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4155/4/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS4, Line 339: Note that we cannot run this test in parallel : # with multiple test vectors because the test modifies and checks a single : # impalad metric. > Was debating with myself about this. I still prefer to run in parallel if w Let's talk in person about this test tomorrow. -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4155/4/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS4, Line 339: Note that we cannot run this test in parallel : # with multiple test vectors because the test modifies and checks a single : # impalad metric. > Given this, should the test be marked to run serially? Was debating with myself about this. I still prefer to run in parallel if we can. This constraint ensures that we'll only ever run this test once. Otoh, I these tests seem more like lib cache stress tests, so maybe it's cleaner to move into a separate .py file altogether. What do you think? -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4155/4/tests/metadata/test_ddl.py File tests/metadata/test_ddl.py: PS4, Line 339: Note that we cannot run this test in parallel : # with multiple test vectors because the test modifies and checks a single : # impalad metric. Given this, should the test be marked to run serially? -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 4: Michael I found a few more bugs by running this test in a loop on exhaustive. Needed some minor fixes, can you please take another quick look? Thanks! -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Hello Michael Brown, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/4155 to look at the new patch set (#4). Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. IMPALA-3491: Use unique database fixture in test_ddl.py. Adds new parametrization to the unique database fixture: - num_dbs: allows creating multiple unique databases at once; the 2nd, 3rd, etc. datbase name is generated by appending "2", "3", etc., to the first database name - sync_ddl: allows creating the dabatases(s) with sync_ddl which is needed by most tests in test_ddl.py Testing: I ran debug/core and debug/exhaustive on HDFS and core/debug on S3. Also ran the test locally in a loop on exhaustive. Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/create-database.test M testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test M testdata/workloads/functional-query/queries/QueryTest/create-table.test M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test M testdata/workloads/functional-query/queries/QueryTest/truncate-table.test M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test M tests/common/parametrize.py M tests/conftest.py M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py M tests/metadata/test_hms_integration.py 13 files changed, 498 insertions(+), 675 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4155/4 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 3: (1 comment) Thanks for the fast turnaround, Michael! http://gerrit.cloudera.org:8080/#/c/4155/3/tests/conftest.py File tests/conftest.py: PS3, Line 284: return first_db_name > I don't really like that we return a db name but more than one DB is create Agreed. This is definitely not an ideal solution but it still seems like an improvement. Step by step :) -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 3: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/4155/3/tests/conftest.py File tests/conftest.py: PS3, Line 284: return first_db_name I don't really like that we return a db name but more than one DB is created if num_dbs > 1, and that tests using the fixture then have to do some string concatenation to derive those DBs' names, and that .test files only know 1 database, and that they have to do the same string concatenation. However, that's a lot of other mess to fix for this patch, so I'm willing to live with that. The .test problem is particularly gross and doesn't have an immediately obvious solution. -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has uploaded a new patch set (#3). Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. IMPALA-3491: Use unique database fixture in test_ddl.py. Allows parametrizing the unique database fixture with sync_ddl which is needed by most tests in test_ddl.py. Testing: I ran debug/core and debug/exhaustive on HDFS and core/debug on S3. Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/create-database.test M testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test M testdata/workloads/functional-query/queries/QueryTest/create-table.test M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test M tests/common/parametrize.py M tests/conftest.py M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py M tests/metadata/test_hms_integration.py 12 files changed, 476 insertions(+), 667 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4155/3 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change.
Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4155/2/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
PS2, Line 59: self.run_stmt_in_hive("create database hms_sanity_db")
> In testing this test, I identified a test flake: this test can't be success
There were two issues preventing this test from being run multiple times in a
row. First, the db should be cleanup up. Second, we need to invalidate metadata
on Impala.
--
To view, visit http://gerrit.cloudera.org:8080/4155
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Michael Brown
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4155/2/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
PS2, Line 59: self.run_stmt_in_hive("create database hms_sanity_db")
In testing this test, I identified a test flake: this test can't be
successfully run twice in a row in a dev environment, because hms_sanity_db
will exist after the test completes, and then fail here after that.
You can reproduce the problem via the following, twice, from the tests/
directory:
impala-py.test -k test_sanity metadata/test_hms_integration.py
One suggestion is to "DROP DATABASE IF EXISTS ... CASCADE" before L59.
--
To view, visit http://gerrit.cloudera.org:8080/4155
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Michael Brown
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4155/1/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:
PS1, Line 263: create external table t_part (i int) partitioned by (j int, s
string)
: location
'$FILESYSTEM_PREFIX/test-warehouse/$DATABASE.db/t_part_tmp';
: alter table t_part add partition (j=cast(2-1 as int), s='2012');
: alter table t_part add if not exists partition (j=1, s='2012');
: alter table t_part add if not exists partition (j=1,
s='2012/withslash');
: alter table t_part add partition (j=1, s=substring('foo2013bar',
4, 8));
> If any of thee queries fail, the client throws a Beeswax exception which is
Done
http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
Line 55: def test_sanity(self, vector):
> I'm not sure our execute_query_expect_success even makes sense, at least fo
Done. Tested by hand and I'm convinced.
--
To view, visit http://gerrit.cloudera.org:8080/4155
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Michael Brown
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has uploaded a new patch set (#2). Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. IMPALA-3491: Use unique database fixture in test_ddl.py. Allows parametrizing the unique database fixture with sync_ddl which is needed by most tests in test_ddl.py. Testing: I ran debug/core and debug/exhaustive on HDFS and core/debug on S3. Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/create-database.test M testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test M testdata/workloads/functional-query/queries/QueryTest/create-table.test M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test M tests/common/parametrize.py M tests/conftest.py M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py M tests/metadata/test_hms_integration.py 12 files changed, 470 insertions(+), 666 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4155/2 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Michael Brown
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has posted comments on this change.
Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/4155/1/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:
PS1, Line 263: create external table t_part (i int) partitioned by (j int, s
string)
: location
'$FILESYSTEM_PREFIX/test-warehouse/$DATABASE.db/t_part_tmp';
: alter table t_part add partition (j=cast(2-1 as int), s='2012');
: alter table t_part add if not exists partition (j=1, s='2012');
: alter table t_part add if not exists partition (j=1,
s='2012/withslash');
: alter table t_part add partition (j=1, s=substring('foo2013bar',
4, 8));
> Will the test framework adequately fail the test if one of these reports an
If any of thee queries fail, the client throws a Beeswax exception which is
raised up to run_test_case, so I'd say the test will fail adequately.
For confirmation, you can look at impala_test_suite.py L310 and following.
http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
PS1, Line 234: assert "ddl_test_db" not in self.all_db_names()
> (I found this by searching for the removed members of TEST_DBS.)
Nice catch! Fixed.
PS1, Line 255: unique_dabatase2 = unique_database + '2'
: self._create_db(unique_dabatase2, sync=True)
> Clever, but it's unfortunate you have to do this. I'm not aware of pytest b
Sure, added a new num_dbs param.
PS1, Line 328: create_fn_stmt = "create function {0}.f() returns int "\
: "location '{1}/libTestUdfs.so'
symbol='NoArgs'"\
: .format(unique_database, WAREHOUSE)
> Tip for here and elsewhere: You can join strings across lines without \ if
Fixed here and in some other places in this file.
I'm not really sure about our preferred style here for these string + format
is, so please keep pointing out examples if you want me to fix them.
http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
Line 55: def test_sanity(self, vector):
> Not your changes, but in case you want to improve: This test doesn't have m
I'm not sure our execute_query_expect_success even makes sense, at least for
Beeswax clients. If a query fails the client throws, so the assert is likewise
never executed.
What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/4155
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Michael Brown
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py.
..
Patch Set 1:
(5 comments)
Eyes began to glaze over at last 10% of test_ddl.py. Here's some other comments
for now.
http://gerrit.cloudera.org:8080/#/c/4155/1/testdata/workloads/functional-query/queries/QueryTest/alter-table.test
File testdata/workloads/functional-query/queries/QueryTest/alter-table.test:
PS1, Line 263: create external table t_part (i int) partitioned by (j int, s
string)
: location
'$FILESYSTEM_PREFIX/test-warehouse/$DATABASE.db/t_part_tmp';
: alter table t_part add partition (j=cast(2-1 as int), s='2012');
: alter table t_part add if not exists partition (j=1, s='2012');
: alter table t_part add if not exists partition (j=1,
s='2012/withslash');
: alter table t_part add partition (j=1, s=substring('foo2013bar',
4, 8));
Will the test framework adequately fail the test if one of these reports an
error? It matters here, because unlike in some cases below, there's no SELECT
at the end of this to implicitly verify they all worked.
http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:
PS1, Line 234: assert "ddl_test_db" not in self.all_db_names()
(I found this by searching for the removed members of TEST_DBS.)
assert unique_database not in self.all_db_names()
PS1, Line 255: unique_dabatase2 = unique_database + '2'
: self._create_db(unique_dabatase2, sync=True)
Clever, but it's unfortunate you have to do this. I'm not aware of pytest being
able to use the same fixture twice (or more) in a test.
Should we parametrize unique_database optionally to return a set of databases
instead of just one? It seems like it could use your incrementing numerical
suffix scheme and be fine.
PS1, Line 328: create_fn_stmt = "create function {0}.f() returns int "\
: "location '{1}/libTestUdfs.so'
symbol='NoArgs'"\
: .format(unique_database, WAREHOUSE)
Tip for here and elsewhere: You can join strings across lines without \ if they
are within parens.
create_fn_stmt = ("create function {0}.f() returns int "
"location '{1}/libTestUdfs.so' symbol='NoArgs'"
.format(unique_database, WAREHOUSE))
http://gerrit.cloudera.org:8080/#/c/4155/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:
Line 55: def test_sanity(self, vector):
Not your changes, but in case you want to improve: This test doesn't have many
asserts. I think it could be improved by replacing the self.client.execute()
statements with self.execute_query_expect_success() statement. L70 for sure is
never verified.
--
To view, visit http://gerrit.cloudera.org:8080/4155
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm
Gerrit-Reviewer: Michael Brown
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Michael Brown has posted comments on this change. Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. Patch Set 1: Alex, I'm ACK'ing that I have seen this review request. -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm Gerrit-Reviewer: Michael Brown Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-3491: Use unique database fixture in test ddl.py.
Alex Behm has uploaded a new change for review. http://gerrit.cloudera.org:8080/4155 Change subject: IMPALA-3491: Use unique database fixture in test_ddl.py. .. IMPALA-3491: Use unique database fixture in test_ddl.py. Allows parametrizing the unique database fixture with sync_ddl which is needed by most tests in test_ddl.py. Testing: I ran debug/core and debug/exhaustive on HDFS and core/debug on S3. Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M testdata/workloads/functional-query/queries/QueryTest/create-database.test M testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test M testdata/workloads/functional-query/queries/QueryTest/create-table.test M testdata/workloads/functional-query/queries/QueryTest/functions-ddl.test M testdata/workloads/functional-query/queries/QueryTest/views-ddl.test M tests/common/parametrize.py M tests/conftest.py M tests/metadata/test_ddl.py M tests/metadata/test_ddl_base.py M tests/metadata/test_hms_integration.py 12 files changed, 444 insertions(+), 648 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/4155/1 -- To view, visit http://gerrit.cloudera.org:8080/4155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Idf667dd5e960768879c019e2037cf48ad4e4241b Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm
