Repository: impala
Updated Branches:
  refs/heads/master c8bfcbd6e -> 424d6d072


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 <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/424d6d07
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/424d6d07
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/424d6d07

Branch: refs/heads/master
Commit: 424d6d072280e1fda6b71aea8ae8d86cea7e592a
Parents: c8bfcbd
Author: Gabor Kaszab <[email protected]>
Authored: Tue Jun 5 18:08:15 2018 +0200
Committer: Impala Public Jenkins <[email protected]>
Committed: Mon Jun 25 10:07:15 2018 +0000

----------------------------------------------------------------------
 .../main/java/org/apache/impala/catalog/HdfsTable.java  | 12 ++++--------
 .../org/apache/impala/planner/HdfsPartitionPruner.java  |  6 ++----
 2 files changed, 6 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/424d6d07/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
----------------------------------------------------------------------
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 895e737..1f20cc8 100644
--- a/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
+++ b/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
@@ -186,9 +186,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.
@@ -587,8 +584,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) {
@@ -746,7 +746,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();
@@ -1094,7 +1093,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) {
@@ -1142,8 +1140,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;

http://git-wip-us.apache.org/repos/asf/impala/blob/424d6d07/fe/src/main/java/org/apache/impala/planner/HdfsPartitionPruner.java
----------------------------------------------------------------------
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

Reply via email to