[impala] 02/14: IMPALA-7121: Clean up partitionIds_ from HdfsTable

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 34e444ca86d51b262df97a8c73b351dea6fb78c6
Author: Gabor Kaszab <ga...@cloudera.com>
AuthorDate: Tue Jun 5 18:08:15 2018 +0200

    IMPALA-7121: Clean up partitionIds_ from HdfsTable
    
    The purpose of introducing partitionIds_ member to HdfsTable was to be
    able to return the IDs of all the current partitions in constant time.
    Apparently, partitionMap_ also contains these IDs as the key of the
    map and this is accessible via keySet() also in constant time. It
    seems reasonable then to remove partitionIds_ and use
    partitionMap_.keySet() in getPartitionIds() to save some memory.
    
    Change-Id: I8b5a480e570aeae565fafd4f3e2b279e7a98c7da
    Reviewed-on: http://gerrit.cloudera.org:8080/10654
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 fe/src/main/java/org/apache/impala/catalog/HdfsTable.java    | 12 ++++--------
 .../java/org/apache/impala/planner/HdfsPartitionPruner.java  |  6 ++----
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
index 13521df..34896cb 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -185,9 +185,6 @@ public class HdfsTable extends Table implements FeFsTable {
   // table metadata loading.
   private final HashMap<String, HdfsPartition> nameToPartitionMap_ = Maps.newHashMap();
 
-  // Store all the partition ids of an HdfsTable.
-  private final HashSet<Long> partitionIds_ = Sets.newHashSet();
-
   // The partition used as a prototype when creating new partitions during
   // insertion. New partitions inherit file format and other settings from
   // the prototype.
@@ -586,8 +583,11 @@ public class HdfsTable extends Table implements FeFsTable {
     return partitionLocationCompressor_;
   }
 
+  // Returns an unmodifiable set of the partition IDs from partitionMap_.
   @Override // FeFsTable
-  public Set<Long> getPartitionIds() { return partitionIds_; }
+  public Set<Long> getPartitionIds() {
+    return Collections.unmodifiableSet(partitionMap_.keySet());
+  }
 
   @Override // FeFsTable
   public TreeMap<LiteralExpr, HashSet<Long>> getPartitionValueMap(int i) {
@@ -745,7 +745,6 @@ public class HdfsTable extends Table implements FeFsTable {
    * Clear the partitions of an HdfsTable and the associated metadata.
    */
   private void resetPartitions() {
-    partitionIds_.clear();
     partitionMap_.clear();
     nameToPartitionMap_.clear();
     partitionValuesMap_.clear();
@@ -1093,7 +1092,6 @@ public class HdfsTable extends Table implements FeFsTable {
    */
   private void updatePartitionMdAndColStats(HdfsPartition partition) {
     if (partition.getPartitionValues().size() != numClusteringCols_) return;
-    partitionIds_.add(partition.getId());
     nameToPartitionMap_.put(partition.getPartitionName(), partition);
     if (!isStoredInImpaladCatalogCache()) return;
     for (int i = 0; i < partition.getPartitionValues().size(); ++i) {
@@ -1141,8 +1139,6 @@ public class HdfsTable extends Table implements FeFsTable {
     Preconditions.checkArgument(partition.getPartitionValues().size() ==
         numClusteringCols_);
     Long partitionId = partition.getId();
-    // Remove the partition id from the list of partition ids and other mappings.
-    partitionIds_.remove(partitionId);
     partitionMap_.remove(partitionId);
     nameToPartitionMap_.remove(partition.getPartitionName());
     if (!isStoredInImpaladCatalogCache()) return partition;
diff --git a/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java b/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
index 07a2636..1584d44 100644
--- a/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
+++ b/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
@@ -276,17 +276,15 @@ public class HdfsPartitionPruner {
     }
     if (op == Operator.DISTINCT_FROM) {
       // Case: SlotRef IS DISTINCT FROM Literal
+      matchingIds.addAll(tbl_.getPartitionIds());
       if (literal instanceof NullLiteral) {
-        matchingIds.addAll(tbl_.getPartitionIds());
         Set<Long> nullIds = tbl_.getNullPartitionIds(partitionPos);
         matchingIds.removeAll(nullIds);
-        return matchingIds;
       } else {
-        matchingIds.addAll(tbl_.getPartitionIds());
         HashSet<Long> ids = partitionValueMap.get(literal);
         if (ids != null) matchingIds.removeAll(ids);
-        return matchingIds;
       }
+      return matchingIds;
     }
     if (op == Operator.NE) {
       // Case: SlotRef != Literal