You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Alex Behm (Code Review)" <ge...@cloudera.org> on 2016/05/12 16:01:06 UTC

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test_ddl.py. Part 1.

Alex Behm has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3044

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................

IMPALA-3530: Clean up test_ddl.py. Part 1.

This is the first in a series of paches to clean up test_ddl.py

Summary of changes:
  - Break up test_create() and corresponding .test files into:
    * test_create_database()
    * test_creae_table()
    * test_create_table_like_table()
    * test_create_table_like_file()
    * test_create_table_as_select()
  - Merge test_nested() into the tests above
  - Move a test into test_gms_integration.py
  - Add a new test_ddl_base.py as base class for DDL tests.
    The plan is to split up test_ddl.py into several smaller
    .py files in subsequent patches.

Testing: I tested test_ddl.py and test_hms_integration.py on
exhaustive locally as well as in private builds on all filesystems.

Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
---
A testdata/workloads/functional-query/queries/QueryTest/create-database.test
D testdata/workloads/functional-query/queries/QueryTest/create-nested.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
A testdata/workloads/functional-query/queries/QueryTest/create-table.test
D testdata/workloads/functional-query/queries/QueryTest/create.test
M tests/metadata/test_ddl.py
A tests/metadata/test_ddl_base.py
M tests/metadata/test_hms_integration.py
10 files changed, 1,185 insertions(+), 1,181 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/44/3044/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 5: Code-Review+2 Verified+1

I validated this with several private builds, merging manually. The hdfs run was on exhaustive:
http://sandbox.jenkins.cloudera.com/view/Impala/view/Private-Utility/job/impala-private-build-and-test/3363/
http://sandbox.jenkins.cloudera.com/view/Impala/view/Private-Utility/job/impala-private-build-and-test-local/7/
http://sandbox.jenkins.cloudera.com/view/Impala/view/Private-Utility/job/impala-private-build-and-test-s3/168/

-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: No

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 2:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/3044/2//COMMIT_MSG
Commit Message:

PS2, Line 14: create_table
> I presume Kudu and HBase will have their own test files? How about tables t
We cannot create HBase tables.
S3 tables are 'HDFS' tables with special hadoop configurations, but no changes to the DDL. Same for localfilesystem tables.

Kudu create tests are separate.

The most accurate name seems to be test_create_non_kudu_tables() which seems weird.

I added a TODO to move the Kudu-specific stuff into a separate pytest. Right now the Kudu tests are spread in various places, and I want to avoid making too many changes in this patch, if possible.


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-database.test
File testdata/workloads/functional-query/queries/QueryTest/create-database.test:

Line 86: # Should drop all tables and the database
> Shouldn't also drop functions and aggregate functions?
Correct. Modified comment.


PS2, Line 95: create database if not exists test_drop_restrict_db
            : ====
            : ---- QUERY
            : show databases like 'test_drop_restrict_db'
            : ---- RESULTS
            : 'test_drop_restrict_db',''
            : ---- TYPES
            : STRING,STRING
            : ====
            : ---- QUERY
            : drop database test_drop_restrict_db restrict
            : ---- RESULTS
            : ====
            : ---- QUERY
            : show databases like 'test_drop_restrict_db'
            : ---- RESULTS
> What are we testing here?
Tests that a drop with RESTRICT executes fine. Added comment.


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
File testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test:

PS2, Line 98: # Validate CTAS with LIMIT 0
            : create table if not exists ctas_join_limit0 stored as textfile as
            : select * from functional.jointbl limit 0
> Duplicate of test in L73?
Hard to tell. It may have a purpose, e.g., to test the behavior when the target
table already exists. I verified that I did not mistakenly add the duplicate, tests were like
this before my patch. I added a comment to clarify the distinction.


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
File testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test:

PS2, Line 59: show table stats like_view
> Do these statements belong here? Maybe putting them in the compute-stats te
This is testing that the table is created in an expected state with "create table like".
We could move that to compute-stats.test, but it's not really testing compute stats.


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-table.test
File testdata/workloads/functional-query/queries/QueryTest/create-table.test:

PS2, Line 3: # It should show up now
> Can you update the comment?
Done


PS2, Line 45: insert overwrite table ddl_test_db.testtbl SELECT 1, 'Hi'
            : from functional.alltypes limit 10
> I am not sure DML statements should be in this or any of the create table l
1. Do you want me to remove those out-of-place statements (here and elsewhere) and lose test coverage?
2. There have been cases in the past where creating a table worked, but performing operations against it did not. I think what we really want to test is that table creation in all possible ways works, that the table is created in an expecte state, and that all possible operations against that table also work (i.e. the table is usable).
3. The ultimate goal is to enable the unique_database fixture to speed up tests.


PS2, Line 226: creatin
> "creation"
Done


http://gerrit.cloudera.org:8080/#/c/3044/2/tests/metadata/test_ddl_base.py
File tests/metadata/test_ddl_base.py:

PS2, Line 23: TEST_DBS = ['ddl_test_db', 'ddl_purge_db', 'alter_table_test_db',
            :               'alter_table_test_db2', 'function_ddl_test', 'udf_test', 'data_src_test',
            :               'truncate_table_test_db', 'test_db', 'alter_purge_db', 'db_with_comment']
> Why does this belong to the super class? I would expect every subclass to d
Moved the setup/teardown parts into the only subclass. I will split them up later if needed.


PS2, Line 60: 'part_data', 't1_tmp1', 't_part_tmp'
> This looks specific to some tests, so I am not sure it belongs here.
Done


PS2, Line 66:  
> .
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3044/3/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

PS3, Line 220: Mov
typo: Move


-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test_ddl.py. Part 1.

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 2: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: No

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test_ddl.py. Part 1.

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3044/1//COMMIT_MSG
Commit Message:

Line 9: paches
nit:typo


Line 14: test_creae_table
nit:typo


Line 19: test_gms_integration
nit:typo


http://gerrit.cloudera.org:8080/#/c/3044/1/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

Line 179: execute_serially
Just curious why we run all these tests serially. May be we can use unique db fixture (instead of ddl_test_db) and remove this?


http://gerrit.cloudera.org:8080/#/c/3044/1/tests/metadata/test_ddl_base.py
File tests/metadata/test_ddl_base.py:

Line 15: logging
I think most of the imports here are unused. logging, shlex,time,getpass,call, skips.


http://gerrit.cloudera.org:8080/#/c/3044/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:

Line 24: logging
Not your change, but this has unused imports too. logging, shlex etc.


-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test_ddl.py. Part 1.

Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 2:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/3044/2//COMMIT_MSG
Commit Message:

Line 14: create_table
I presume Kudu and HBase will have their own test files? How about tables that live on S3? Do these test files cover these cases? If not, it may make sense to change the naming so that it reflects what exactly is tested in each file; maybe test_create_hdfs_table or something along these lines.


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-database.test
File testdata/workloads/functional-query/queries/QueryTest/create-database.test:

Line 86: # Should drop all tables and the database
Shouldn't also drop functions and aggregate functions?


Line 95: create database if not exists test_drop_restrict_db
       : ====
       : ---- QUERY
       : show databases like 'test_drop_restrict_db'
       : ---- RESULTS
       : 'test_drop_restrict_db',''
       : ---- TYPES
       : STRING,STRING
       : ====
       : ---- QUERY
       : drop database test_drop_restrict_db restrict
       : ---- RESULTS
       : ====
       : ---- QUERY
       : show databases like 'test_drop_restrict_db'
       : ---- RESULTS
What are we testing here?


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
File testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test:

Line 98: # Validate CTAS with LIMIT 0
       : create table if not exists ctas_join_limit0 stored as textfile as
       : select * from functional.jointbl limit 0
Duplicate of test in L73?


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
File testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test:

Line 59: show table stats like_view
Do these statements belong here? Maybe putting them in the compute-stats tests?


http://gerrit.cloudera.org:8080/#/c/3044/2/testdata/workloads/functional-query/queries/QueryTest/create-table.test
File testdata/workloads/functional-query/queries/QueryTest/create-table.test:

Line 3: # It should show up now
Can you update the comment?


Line 45: insert overwrite table ddl_test_db.testtbl SELECT 1, 'Hi'
       : from functional.alltypes limit 10
I am not sure DML statements should be in this or any of the create table like test files. What I expect from this file is testing all possible ways to call create table and then doing some simple verification to ensure that: 1) the tables are successfully created and 2) that the specified properties are applied.


Line 226: creatin
"creation"


http://gerrit.cloudera.org:8080/#/c/3044/2/tests/metadata/test_ddl_base.py
File tests/metadata/test_ddl_base.py:

Line 23: TEST_DBS = ['ddl_test_db', 'ddl_purge_db', 'alter_table_test_db',
       :               'alter_table_test_db2', 'function_ddl_test', 'udf_test', 'data_src_test',
       :               'truncate_table_test_db', 'test_db', 'alter_purge_db', 'db_with_comment']
Why does this belong to the super class? I would expect every subclass to define the TEST_DBS that it uses.


Line 60: 'part_data', 't1_tmp1', 't_part_tmp'
This looks specific to some tests, so I am not sure it belongs here.


Line 66:  
.


-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has submitted this change and it was merged.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


IMPALA-3530: Clean up test_ddl.py. Part 1.

This is the first in a series of patches to clean up test_ddl.py

Summary of changes:
  - Break up test_create() and corresponding .test files into:
    * test_create_database()
    * test_create_table()
    * test_create_table_like_table()
    * test_create_table_like_file()
    * test_create_table_as_select()
  - Merge test_nested() into the tests above
  - Move a test into test_hms_integration.py
  - Add a new test_ddl_base.py as base class for DDL tests.
    The plan is to split up test_ddl.py into several smaller
    .py files in subsequent patches.

Testing: I tested test_ddl.py and test_hms_integration.py on
exhaustive locally as well as in private builds on all filesystems.

Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Reviewed-on: http://gerrit.cloudera.org:8080/3044
Reviewed-by: Alex Behm <al...@cloudera.com>
Tested-by: Alex Behm <al...@cloudera.com>
---
A testdata/workloads/functional-query/queries/QueryTest/create-database.test
D testdata/workloads/functional-query/queries/QueryTest/create-nested.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
A testdata/workloads/functional-query/queries/QueryTest/create-table.test
D testdata/workloads/functional-query/queries/QueryTest/create.test
M tests/metadata/test_ddl.py
A tests/metadata/test_ddl_base.py
M tests/metadata/test_hms_integration.py
10 files changed, 1,179 insertions(+), 1,181 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved; Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3044

to look at the new patch set (#3).

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................

IMPALA-3530: Clean up test_ddl.py. Part 1.

This is the first in a series of patches to clean up test_ddl.py

Summary of changes:
  - Break up test_create() and corresponding .test files into:
    * test_create_database()
    * test_create_table()
    * test_create_table_like_table()
    * test_create_table_like_file()
    * test_create_table_as_select()
  - Merge test_nested() into the tests above
  - Move a test into test_hms_integration.py
  - Add a new test_ddl_base.py as base class for DDL tests.
    The plan is to split up test_ddl.py into several smaller
    .py files in subsequent patches.

Testing: I tested test_ddl.py and test_hms_integration.py on
exhaustive locally as well as in private builds on all filesystems.

Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
---
A testdata/workloads/functional-query/queries/QueryTest/create-database.test
D testdata/workloads/functional-query/queries/QueryTest/create-nested.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
A testdata/workloads/functional-query/queries/QueryTest/create-table.test
D testdata/workloads/functional-query/queries/QueryTest/create.test
M tests/metadata/test_ddl.py
A tests/metadata/test_ddl_base.py
M tests/metadata/test_hms_integration.py
10 files changed, 1,179 insertions(+), 1,181 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/44/3044/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Dimitris Tsirogiannis,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/3044

to look at the new patch set (#4).

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................

IMPALA-3530: Clean up test_ddl.py. Part 1.

This is the first in a series of patches to clean up test_ddl.py

Summary of changes:
  - Break up test_create() and corresponding .test files into:
    * test_create_database()
    * test_create_table()
    * test_create_table_like_table()
    * test_create_table_like_file()
    * test_create_table_as_select()
  - Merge test_nested() into the tests above
  - Move a test into test_hms_integration.py
  - Add a new test_ddl_base.py as base class for DDL tests.
    The plan is to split up test_ddl.py into several smaller
    .py files in subsequent patches.

Testing: I tested test_ddl.py and test_hms_integration.py on
exhaustive locally as well as in private builds on all filesystems.

Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
---
A testdata/workloads/functional-query/queries/QueryTest/create-database.test
D testdata/workloads/functional-query/queries/QueryTest/create-nested.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
A testdata/workloads/functional-query/queries/QueryTest/create-table.test
D testdata/workloads/functional-query/queries/QueryTest/create.test
M tests/metadata/test_ddl.py
A tests/metadata/test_ddl_base.py
M tests/metadata/test_hms_integration.py
10 files changed, 1,179 insertions(+), 1,181 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/44/3044/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test_ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/3044/1//COMMIT_MSG
Commit Message:

Line 9: This is the first in a series of paches to clean up test_ddl.py
> nit:typo
Done


Line 14:     * test_creae_table()
> nit:typo
Done


Line 19:   - Move a test into test_gms_integration.py
> nit:typo
Done


http://gerrit.cloudera.org:8080/#/c/3044/1/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

Line 179:   @pytest.mark.execute_serially
> Just curious why we run all these tests serially. May be we can use unique 
That's the plan. My goal is to roll out the unique_database fixture in this test, but it is too painful without first cleaning/splitting up this test. So I'm starting with the cleanup, deliberately leaving the unique_database change for later.

(Also see JIRA description)


http://gerrit.cloudera.org:8080/#/c/3044/1/tests/metadata/test_ddl_base.py
File tests/metadata/test_ddl_base.py:

Line 15: import logging
> I think most of the imports here are unused. logging, shlex,time,getpass,ca
Done


http://gerrit.cloudera.org:8080/#/c/3044/1/tests/metadata/test_hms_integration.py
File tests/metadata/test_hms_integration.py:

Line 24: import logging
> Not your change, but this has unused imports too. logging, shlex etc.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................


Patch Set 3:

(1 comment)

I will run a final round of tests.

http://gerrit.cloudera.org:8080/#/c/3044/3/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

PS3, Line 220: Mov
> typo: Move
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-CR](cdh5-trunk) IMPALA-3530: Clean up test_ddl.py. Part 1.

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has uploaded a new patch set (#2).

Change subject: IMPALA-3530: Clean up test_ddl.py. Part 1.
......................................................................

IMPALA-3530: Clean up test_ddl.py. Part 1.

This is the first in a series of patches to clean up test_ddl.py

Summary of changes:
  - Break up test_create() and corresponding .test files into:
    * test_create_database()
    * test_create_table()
    * test_create_table_like_table()
    * test_create_table_like_file()
    * test_create_table_as_select()
  - Merge test_nested() into the tests above
  - Move a test into test_hms_integration.py
  - Add a new test_ddl_base.py as base class for DDL tests.
    The plan is to split up test_ddl.py into several smaller
    .py files in subsequent patches.

Testing: I tested test_ddl.py and test_hms_integration.py on
exhaustive locally as well as in private builds on all filesystems.

Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
---
A testdata/workloads/functional-query/queries/QueryTest/create-database.test
D testdata/workloads/functional-query/queries/QueryTest/create-nested.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test
A testdata/workloads/functional-query/queries/QueryTest/create-table.test
D testdata/workloads/functional-query/queries/QueryTest/create.test
M tests/metadata/test_ddl.py
A tests/metadata/test_ddl_base.py
M tests/metadata/test_hms_integration.py
10 files changed, 1,176 insertions(+), 1,188 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/44/3044/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3044
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>