impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5333: Add support for Impala to work with ADLS
Date Wed, 24 May 2017 18:51:07 GMT
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 <sailesh@cloudera.com>
Gerrit-Reviewer: Attila Jeges <attilaj@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message