Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22014 )

Change subject: IMPALA-13154: Update stats when loading an HDFS table
......................................................................


Patch Set 10: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/22014/10/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/22014/10/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@765
PS10, Line 765:   private FileMetadataStats fileMetadataStats_ = null;
nit: Can we add some comments like other fields? E.g. saying that these are 
stats corresponding to encoded file descriptors.


http://gerrit.cloudera.org:8080/#/c/22014/10/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1277
PS10, Line 1277:   // NOTE: A Builder object should not be re-used. That is, 
once the build() method is
               :   // called, it should not be called again on the same object.
nit: We can add a precondition check in build() and add a boolean field to 
check whether it's built.

BTW, what's the problem if build() is used twice?


http://gerrit.cloudera.org:8080/#/c/22014/9/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/22014/9/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@363
PS9, Line 363:     // Initializes the values of the storage stats.
             :     public void init() {
             :       numFiles = 0;
             :       numBlocks = 0;
             :       totalFileBytes = 0;
             :     }
> Thanks! Removed a call in updateStatistics(). But it seems that init() is a
I see. That makes sense to keep the init() method.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e2eb503b0f61b1e6403058bc5dc78d721e7e940
Gerrit-Change-Number: 22014
Gerrit-PatchSet: 10
Gerrit-Owner: Xuebin Su <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Xuebin Su <[email protected]>
Gerrit-Comment-Date: Mon, 11 Nov 2024 00:20:14 +0000
Gerrit-HasComments: Yes

Reply via email to