impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <>
Subject [Impala-CR](cdh5-trunk) IMPALA-3530: Clean up Part 1.
Date Wed, 01 Jun 2016 20:05:20 GMT
Dimitris Tsirogiannis has posted comments on this change.

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

Patch Set 2:

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.
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
       : ====
       : ---- QUERY
       : drop database test_drop_restrict_db restrict
       : ---- RESULTS
       : ====
       : ---- QUERY
       : show databases like 'test_drop_restrict_db'
       : ---- RESULTS
What are we testing here?
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?
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?
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
File tests/metadata/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f4c044d39e165c2535961b8d0a765c8dbbd051c
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Bharath Vissapragada <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-HasComments: Yes

View raw message