Sudhanshu Arora has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13215 )

Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and 
partition
......................................................................


Patch Set 6:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:

http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@482
PS6, Line 482:   // 
<table_name>:<highwatermark>:<minOpenWriteId>:<open_writeids>:<abort_writeids>
Are we guaranteed that Hive team will keep this string backward compatible?


http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java@231
PS6, Line 231:     return null;
throw UnsupportedOperationException.


http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:

http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@374
PS6, Line 374:   * @return the list of valid write IDs for the table in a string
Nit: or null if there are no validWriteIds


http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:

http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@236
PS6, Line 236:         StringBuilder validIdsBuf = new StringBuilder("Loaded 
ValidWriteIdLists: ");
For my understanding, how do we use timeline?


http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:

http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@649
PS6, Line 649:     writeId_ = msPartition != null ?
Nit: Handle null case in shim so that every call does not have to handle it.


http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1026
PS6, Line 1026:     }
Nit: Use ternary or put else in the above line



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Sudhanshu Arora <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]>
Gerrit-Reviewer: Yongzhi Chen <[email protected]>
Gerrit-Comment-Date: Mon, 06 May 2019 23:48:48 +0000
Gerrit-HasComments: Yes

Reply via email to