impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Brown (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5287: Test skip.header.line.count on gzip
Date Mon, 08 May 2017 19:21:32 GMT
Michael Brown has posted comments on this change.

Change subject: IMPALA-5287: Test skip.header.line.count on gzip
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6817/1/tests/query_test/test_scanners.py
File tests/query_test/test_scanners.py:

PS1, Line 688: 
> Good catch. I want to use the default databases for the scans, so that the 
I didn't realize there was magic that changed the default database depending on fileformat/encoding/compression/etc.
I was going to suggest you preserve the unique_database usage and access the 'mixed' table
via full path: "$DATABASE.mixed". But I just looked at the code, and use_db is overloaded
to deal with both what you want to use, and what I was going to suggest, and both aren't possible
together, I think.

What you really need is a way to pass in another database to be used in substitution in this
test and access it in the test files, say "$DB2.mixed".

Idea:

What do you think about adding a new parameter to run_test_case() that takes in a dictionary,
where the keys are testfile substitution variables and the values are the value to be substituted?
This means you don't add a separate parameter "db2" to run_test_suite() but instead support
arbitrary substitution.

The literal way this would be done would be:

  def test_text_scanner_with_header(self, vector, unique_database):
    self.run_test_case(
        'QueryTest/hdfs-text-scan-with-header',
        vector,
        testfile_vars={
          '$DB2': unique_database,
        })

run_test_case() would just iterate over the keys and values in testfile_vars, and your testfile
now can specify any variables it wants for substitution. Your default database will still
be correct based on file format etc. You refer to the mixed table as "$DB2.mixed".


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3f3c29a42501cfb2751f7ad0af166eb88f63b70
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message