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

Reply via email to