Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/17538 )
Change subject: IMPALA-10724: Add mutable validWriteIdList ...................................................................... Patch Set 4: Code-Review+2 (3 comments) http://gerrit.cloudera.org:8080/#/c/17538/4/fe/src/main/java/org/apache/impala/hive/common/MutableValidReaderWriteIdList.java File fe/src/main/java/org/apache/impala/hive/common/MutableValidReaderWriteIdList.java: http://gerrit.cloudera.org:8080/#/c/17538/4/fe/src/main/java/org/apache/impala/hive/common/MutableValidReaderWriteIdList.java@112 PS4, Line 112: if (exceptions.size() == 0) { : buf.append(':'); : buf.append(':'); : } nit: this is covered by the else-clause so can be removed. http://gerrit.cloudera.org:8080/#/c/17538/4/fe/src/main/java/org/apache/impala/hive/common/MutableValidReaderWriteIdList.java@146 PS4, Line 146: String[] values = src.split(":"); Could you add a precondition check like this? Preconditions.checkState(values.length >= 3, "Not enough values"); http://gerrit.cloudera.org:8080/#/c/17538/4/fe/src/main/java/org/apache/impala/hive/common/MutableValidReaderWriteIdList.java@171 PS4, Line 171: int i = 0; nit: unused var -- To view, visit http://gerrit.cloudera.org:8080/17538 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28e60db0afd5d4398af24449b72abc928421f7c6 Gerrit-Change-Number: 17538 Gerrit-PatchSet: 4 Gerrit-Owner: Yu-Wen Lai <yu-wen....@cloudera.com> Gerrit-Reviewer: Anonymous Coward <kis...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Yu-Wen Lai <yu-wen....@cloudera.com> Gerrit-Comment-Date: Mon, 12 Jul 2021 06:58:31 +0000 Gerrit-HasComments: Yes
