Vihang Karajgaonkar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16008 )

Change subject: IMPALA-9791: Support validWriteIdList in 
getPartialCatalogObject API
......................................................................


Patch Set 15:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16008/14/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/16008/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@20
PS14, Line 20: import java.io.FileNotFoundException
> nit: move this to line 102
Done


http://gerrit.cloudera.org:8080/#/c/16008/14/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1552
PS14, Line 1552: "Could not use
> We should not use String.format() here since we are using "{}" instead of "
yes, makes sense. Thanks!


http://gerrit.cloudera.org:8080/#/c/16008/14/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java
File 
fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java:

http://gerrit.cloudera.org:8080/#/c/16008/14/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@21
PS14, Line 21: import java.sql.SQLException;
> nit: can remove them since the following codes are not using these static i
Done


http://gerrit.cloudera.org:8080/#/c/16008/14/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@42
PS14, Line 42: import org.junit.After;
> nit: unused import
Done


http://gerrit.cloudera.org:8080/#/c/16008/14/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@223
PS14, Line 223:     ValidWriteIdList currentWriteIdList = 
getValidWriteIdList(testDbName,
              :       testPartitionedTbl);
> Can we simply get this by tbl.getValidWriteIds() instead of sending an HMS
I think its safer to get the validwriteIdList directly from the source of truth 
for the test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied2c7c3cb2009c407e8fbc3af4722b0d34f57c4a
Gerrit-Change-Number: 16008
Gerrit-PatchSet: 15
Gerrit-Owner: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Anurag Mantripragada <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Thu, 11 Jun 2020 05:43:25 +0000
Gerrit-HasComments: Yes

Reply via email to