Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16516 )
Change subject: IMPALA-10189: don't reload partitions for stat operations ...................................................................... Patch Set 1: (2 comments) I had a suggested alternative to the bools, lmk what you think. http://gerrit.cloudera.org:8080/#/c/16516/1/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/16516/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@486 PS1, Line 486: public void markDirtyPartitions(Collection<HdfsPartition.Builder> partBuilders) { > Change to Collection& and consider making entries const. I don't think this makes sense in Java? http://gerrit.cloudera.org:8080/#/c/16516/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/16516/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4066 PS1, Line 4066: Preconditions.checkState(!markDirty || !updatePartitions, > Maybe a little cleaner just to use one boolean flag? It's valid to have both false if the caller marks partitions dirty. I could replace the bools with an enum with three values though if you think that'll be clearer (I'm kinda on the fence) -- To view, visit http://gerrit.cloudera.org:8080/16516 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I91009d6e9fdf25b3a3341fd1d29219519eb39a3d Gerrit-Change-Number: 16516 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Tue, 29 Sep 2020 17:02:20 +0000 Gerrit-HasComments: Yes
