Bharath Vissapragada has posted comments on this change. Change subject: IMPALA-5615: Fix compute incremental stats for general partition exprs ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/7379/1/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java: Line 361: Sets.newHashSet(partitionSet_.getPartitions()); > I don't fully understand the motivation for this part of the change. I added this for having O(1) lookups on L374. Given the partitionSet_ can now return arbitrary number of partitions, I didn't want it to be an O(n^2) loop. Good point about the LinkedHashSet, I made that change. > Does getPartitions() return duplicates? If so, it might be better to add a > getUniquePartitions() method to PartitionSet I don't think it can return duplicate partitions. Reason being we don't add duplicate partitions in the first place. If the PartitionSpec matches with any of the existing partitions, we return an error [1. I'm pretty sure there are other codepaths that add partitions to an HdfsTable but I don't think duplicates are possible. Not sure if that answers your question, please let me know if I'm getting it wrong. [1] https://github.com/apache/incubator-impala/blob/master/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#L1924 -- To view, visit http://gerrit.cloudera.org:8080/7379 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I227fc06f580eb9174f60ad0f515a3641cec19268 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
