Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11027 )

Change subject: IMPALA-7320. Avoid calling getFileStatus() for each partition 
when table is loaded
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:

http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@857
PS1, Line 857:         HdfsPartition partition = 
createPartition(msPartition.getSd(), msPartition, permCache);
> longline
Done


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1005
PS1, Line 1005: );
> Can you add the table name too.
Done


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1015
PS1, Line 1015:    * Throws CatalogException if one of the supplied storage 
descriptors contains metadata that
> nit: longline
Done


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1026
PS1, Line 1026:       HdfsPartition hdfsPartition = 
createPartition(partition.getSd(), partition, permCache);
> longline
Done


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1625
PS1, Line 1625:       HdfsPartition partition = 
createPartition(msPartition.getSd(), msPartition, permCache);
> longline
Done


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1675
PS1, Line 1675: );
> , ioe);
Done


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java
File fe/src/main/java/org/apache/impala/util/FsPermissionCache.java:

http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java@40
PS1, Line 40: FileSystem
> Technically not all paths need to be fully-qualified. I'll see if we can re
I looked through the source for FileStatus.getPath() and it seems to "do the 
right thing" here -- if the path that we listed is qualified, it returns a 
similarly-qualified path on the resulting children, so things should match up 
appropriately without having to include the FileSystem here.


http://gerrit.cloudera.org:8080/#/c/11027/1/fe/src/main/java/org/apache/impala/util/FsPermissionCache.java@45
PS1, Line 45: if (perms != null) {
            :       return perms;
            :     }
> single line.
Done



--
To view, visit http://gerrit.cloudera.org:8080/11027
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I83e5ebc214d6620d165e13f8cc80f8fdda100734
Gerrit-Change-Number: 11027
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Bharath Vissapragada <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tianyi Wang <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Vuk Ercegovac <[email protected]>
Gerrit-Comment-Date: Tue, 24 Jul 2018 18:38:56 +0000
Gerrit-HasComments: Yes

Reply via email to