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 <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
