Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10543 )
Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same location ...................................................................... Patch Set 6: (2 comments) Just one more thing to clarify, if we have 2 different tables which each have one of their partitions pointing to the same location, this current approach won't handle that right? If yes, we may want to document that we don't take care of that case. http://gerrit.cloudera.org:8080/#/c/10543/2/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/10543/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1485 PS2, Line 1485: if (partitions == null) { > I did a short research and I found that HashSet.keySet() is O(1) as it retu Thanks Bharath and Gabor for digging in. I was just asking some open questions to find opportunities for making some of this more cleaner, and wasn't suggesting that we change it in this patch. keySet() is pretty useful. I didn't know the Java map had such an API. But as you mentioned, it would be nice to force the returned keySet to be immutable or read only. Let's do that in a different patch. w.r.t the 'locationToPartMap_', if no one else has any more suggestions, I think we can move forward with Gabor's current implementation and revisit after some refactoring is done in this class. http://gerrit.cloudera.org:8080/#/c/10543/3/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/10543/3/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1167 PS3, Line 1167: removePartitionFromLocationMapping(partition); > Nothing extra has to be done here for the following reason: I see, so it's on the caller to decide what all partitions to drop. What about the call from L1440 in updatePartitionsFromHms()? That drops partitions that were externally dropped in Hive Metastore, so don't we need to collect all partitions that share the same location in that case too? -- To view, visit http://gerrit.cloudera.org:8080/10543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a Gerrit-Change-Number: 10543 Gerrit-PatchSet: 6 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 05 Jun 2018 17:30:01 +0000 Gerrit-HasComments: Yes
