impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5615: Fix compute incremental stats for general partition exprs
Date Thu, 20 Jul 2017 22:20:10 GMT
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 <bharathv@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message