Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-5333: Add support for Impala to work with ADLS ......................................................................
Patch Set 5: (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: // TODO: Disable permission checking for S3A as well (HADOOP-13892) > This was a longstanding 'bug' even for S3A. I'll fix it for that in a separ You can't include the S3FileSystem in the initialization of shouldCheckPerms? Also, can you summarize in the function comment what kind of permission checking we do or don't do for each of the supported file systems? We also need to make sure we document any limitations of the ADL integration and how it differs from the integration with S3. 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. 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: @SkipIfADLS.hdfs_block_size Hm, why this doesn't work for ADLS? -- 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: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: David Knupp <dkn...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-HasComments: Yes