Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10712 )
Change subject: IMPALA-7140 (part 2). Create skeleton for LocalFsTable ...................................................................... Patch Set 2: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@82 PS2, Line 82: Hdfs sorry about not catching this before, but pls add a todo (assign it to me) to change the "Hdfs" to "Fs" for consistent naming with this interface. looking at all the places where this is used, the misalignment stands out. http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@164 PS2, Line 164: public Map<Long, List<FileDescriptor>> getFilesSample(Collection<? extends FeFsPartition> inputParts, nit: too long line http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java: http://gerrit.cloudera.org:8080/#/c/10712/2/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@95 PS2, Line 95: "__HIVE_DEFAULT_PARTITION__" make this a public member so the test can use it as well. -- To view, visit http://gerrit.cloudera.org:8080/10712 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id2b184104d92e128250df5a08ac7ffb3dde011a8 Gerrit-Change-Number: 10712 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Thu, 14 Jun 2018 21:21:10 +0000 Gerrit-HasComments: Yes
