impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sailesh Mukil (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems
Date Thu, 07 Apr 2016 21:18:16 GMT
Sailesh Mukil has posted comments on this change.

Change subject: IMPALA-1878: Support INSERT and LOAD DATA on S3 and between filesystems
......................................................................


Patch Set 11:

(7 comments)

That sounds good to me. But just FYI, Anuj is on PTO today and tomorrow. Which means we will
need to wait till Monday till we can get this in.

http://gerrit.cloudera.org:8080/#/c/2574/11/testdata/workloads/functional-query/queries/QueryTest/insert_permutation.test
File testdata/workloads/functional-query/queries/QueryTest/insert_permutation.test:

Line 4: '$FILESYSTEM_PREFIX/test-warehouse/insert_premutation_test'
> typo in the location.
Good catch. Yes, we'll no longer need to explicitly specify FILESYSTEM_PREFIX after the defaultFS
patch.


http://gerrit.cloudera.org:8080/#/c/2574/11/testdata/workloads/functional-query/queries/QueryTest/multiple-filesystems.test
File testdata/workloads/functional-query/queries/QueryTest/multiple-filesystems.test:

Line 33
> Are we losing coverage by not having a test that tries to INSERT into a non
We are losing out on testing the path that returns an error if we use an unsupported filesystem
during an INSERT.
However, we don't have a filesystem now that we can CREATE a table in but not INSERT.


http://gerrit.cloudera.org:8080/#/c/2574/11/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:

Line 120:     # Filesystem client acts as a generic client to supported filesystems.
> Comment is a bit redundant.
Done


http://gerrit.cloudera.org:8080/#/c/2574/11/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

Line 70:   @SkipIfS3.qualified_path
> FWIW, I just filed https://issues.cloudera.org/browse/IMPALA-3313 to deal w
Yes, that sounds like a much better opt-in/out process than what we have now.


http://gerrit.cloudera.org:8080/#/c/2574/11/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:

Line 160:   @SkipIfS3.jira
> What's the JIRA?
It hits IMPALA-551 sometimes. But I just realized that I don't need to skip it for that. It
gets xfailed automatically.


http://gerrit.cloudera.org:8080/#/c/2574/11/tests/util/filesystem_base.py
File tests/util/filesystem_base.py:

Line 1: 
> Remove blank line
Done


http://gerrit.cloudera.org:8080/#/c/2574/11/tests/util/filesystem_utils.py
File tests/util/filesystem_utils.py:

Line 32: speicfic
> specific
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94e15ad67752dce21c9b7c1dced6e114905a942d
Gerrit-PatchSet: 11
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Michael Brown <mikeb@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message