Sailesh Mukil has posted comments on this change. Change subject: IMPALA-5333: Add support for Impala to work with ADLS ......................................................................
Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/6910/5/fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java File fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java: Line 155: // it. If the source file is a file, we must be able to read from it, and write to > You can't include the S3FileSystem in the initialization of shouldCheckPerm We can do it for S3AFileSystem here too, but I'd like to treat that as a separate patch since if we see any issues with that, it would be easier to track down, and I'd also like to do separate S3 testing with that fix. I've added a comment about the permission checking for the different filesystems. And yes, that's a good point. I'll make sure to document the limitations of the AdlFileSystem. http://gerrit.cloudera.org:8080/#/c/6910/5/fe/src/main/java/org/apache/impala/common/FileSystemUtil.java File fe/src/main/java/org/apache/impala/common/FileSystemUtil.java: PS5, Line 305: return !(fs instanceof S3AFileSystem || fs instanceof LocalFileSystem || : fs instanceof AdlFileSystem); : } > nit: I don't think there was anything wrong with the indentation here. Done http://gerrit.cloudera.org:8080/#/c/6910/5/tests/query_test/test_insert_parquet.py File tests/query_test/test_insert_parquet.py: PS5, Line 164: # ADLS does not have a conf > Hm, why this doesn't work for ADLS? There was a comment about this earlier too where I've given an explanation. I added a comment above the test too to avoid future confusion. S3 has a config option to control the "fake" block size. ADLS currently does not have such an option. The 'PARQUET_FILE_SIZE' is passed into hdfsFileOpen() as the block size to be used while writing into files. This is not respected by ADLS since they use a dummy value for ADLS block sizes that's defaulted to 256MB. Currently the ADLS folks have no plans to expose their internal block sizes, so I haven't opened a JIRA for it. This test tries to set the block size to 40MB and see if it works, but since ADLS won't respect that option, this test fails. -- To view, visit http://gerrit.cloudera.org:8080/6910 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic56b9988b32a330443f24c44f9cb2c80842f7542 Gerrit-PatchSet: 6 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-HasComments: Yes
