You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by pa...@apache.org on 2011/07/06 19:49:16 UTC

svn commit: r1143508 - in /hive/trunk: metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java

Author: pauly
Date: Wed Jul  6 17:49:15 2011
New Revision: 1143508

URL: http://svn.apache.org/viewvc?rev=1143508&view=rev
Log:
HIVE-2219. Make "alter table drop partition" more efficient (Sohan Jain via pauly)


Modified:
    hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java
    hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java

Modified: hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java
URL: http://svn.apache.org/viewvc/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java?rev=1143508&r1=1143507&r2=1143508&view=diff
==============================================================================
--- hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java (original)
+++ hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java Wed Jul  6 17:49:15 2011
@@ -27,6 +27,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.TreeMap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -277,17 +278,20 @@ public class Warehouse {
   }
 
   /**
-   * Makes a partition name from a specification
-   * @param spec
-   * @param addTrailingSeperator if true, adds a trailing separator e.g. 'ds=1/'
-   * @return
+   * A common function for constructing a partition name
+   * @param spec the partition spec
+   * @param addTrailingSeparator whether to add a trailing separator at the end
+   * @param isSorted whether the partition name should be sorted by key
+   * @return a partition name
    * @throws MetaException
    */
-  public static String makePartName(Map<String, String> spec,
-      boolean addTrailingSeperator)
-      throws MetaException {
+  private static String makePartNameCommon(Map<String, String> spec,
+      boolean addTrailingSeparator, boolean isSorted) throws MetaException {
     StringBuilder suffixBuf = new StringBuilder();
     int i = 0;
+    if (isSorted) {
+      spec = new TreeMap<String, String>(spec);
+    }
     for (Entry<String, String> e : spec.entrySet()) {
       if (e.getValue() == null || e.getValue().length() == 0) {
         throw new MetaException("Partition spec is incorrect. " + spec);
@@ -300,11 +304,39 @@ public class Warehouse {
       suffixBuf.append(escapePathName(e.getValue()));
       i++;
     }
-    if (addTrailingSeperator) {
+    if (addTrailingSeparator) {
       suffixBuf.append(Path.SEPARATOR);
     }
     return suffixBuf.toString();
   }
+
+  /**
+   * Makes a partition name from a specification
+   * @param spec
+   * @param addTrailingSeperator if true, adds a trailing separator e.g. 'ds=1/'
+   * @return
+   * @throws MetaException
+   */
+  public static String makePartName(Map<String, String> spec,
+      boolean addTrailingSeperator)
+      throws MetaException {
+    return makePartNameCommon(spec, addTrailingSeperator, false);
+  }
+
+  /**
+   * Makes a partition name from a specification.  The keys/value pairs in the partition
+   * name are sorted by key name.  E.g., created=4/ds=1/hr=3.  Sorting the keys is useful
+   * for comparison of partition names.
+   * @param spec the partition spec
+   * @param addTrailingSeparator whether to add a trailing separator at the end of the name
+   * @return a partition name in which the keys are sorted
+   * @throws MetaException
+   */
+  public static String makeSortedPartName(Map<String, String> spec,
+      boolean addTrailingSeparator) throws MetaException {
+    return makePartNameCommon(spec, addTrailingSeparator, true);
+  }
+
   /**
    * Given a dynamic partition specification, return the path corresponding to the
    * static part of partition specification. This is basically a copy of makePartName
@@ -421,5 +453,4 @@ public class Warehouse {
     values.addAll(partSpec.values());
     return values;
   }
-
 }

Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
URL: http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java?rev=1143508&r1=1143507&r2=1143508&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java (original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java Wed Jul  6 17:49:15 2011
@@ -34,14 +34,14 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.Date;
-import java.util.HashSet;
+import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
-import java.util.Map.Entry;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -97,6 +97,7 @@ import org.apache.hadoop.hive.ql.plan.Ad
 import org.apache.hadoop.hive.ql.plan.AlterDatabaseDesc;
 import org.apache.hadoop.hive.ql.plan.AlterIndexDesc;
 import org.apache.hadoop.hive.ql.plan.AlterTableDesc;
+import org.apache.hadoop.hive.ql.plan.AlterTableDesc.AlterTableTypes;
 import org.apache.hadoop.hive.ql.plan.AlterTableSimpleDesc;
 import org.apache.hadoop.hive.ql.plan.CreateDatabaseDesc;
 import org.apache.hadoop.hive.ql.plan.CreateIndexDesc;
@@ -129,7 +130,6 @@ import org.apache.hadoop.hive.ql.plan.Sh
 import org.apache.hadoop.hive.ql.plan.ShowTablesDesc;
 import org.apache.hadoop.hive.ql.plan.SwitchDatabaseDesc;
 import org.apache.hadoop.hive.ql.plan.UnlockTableDesc;
-import org.apache.hadoop.hive.ql.plan.AlterTableDesc.AlterTableTypes;
 import org.apache.hadoop.hive.ql.plan.api.StageType;
 import org.apache.hadoop.hive.ql.security.authorization.Privilege;
 import org.apache.hadoop.hive.serde.Constants;
@@ -162,6 +162,7 @@ public class DDLTask extends Task<DDLWor
   private static String INTERMEDIATE_ORIGINAL_DIR_SUFFIX;
   private static String INTERMEDIATE_EXTRACTED_DIR_SUFFIX;
 
+  @Override
   public boolean requireLock() {
     return this.work != null && this.work.getNeedLock();
   }
@@ -2926,13 +2927,16 @@ public class DDLTask extends Task<DDLWor
           dropTbl.getExpectView());
       }
 
-      // get all partitions of the table
+      // get all partition names of the table
       List<String> partitionNames =
         db.getPartitionNames(dropTbl.getTableName(), (short) -1);
-      Set<Map<String, String>> partitions = new HashSet<Map<String, String>>();
+      //Build a hashtable of a "sorted partition name" to the original partition spec
+      //We need a sorted partition name so we can compare partitions by their names
+      Hashtable<String,Map<String,String>> partitions = new Hashtable<String,Map<String,String>>();
       for (String partitionName : partitionNames) {
         try {
-          partitions.add(Warehouse.makeSpecFromName(partitionName));
+          Map<String, String> spec = Warehouse.makeSpecFromName(partitionName);
+          partitions.put(Warehouse.makeSortedPartName(spec, false), spec);
         } catch (MetaException e) {
           LOG.warn("Unrecognized partition name from metastore: " + partitionName);
         }
@@ -2940,28 +2944,26 @@ public class DDLTask extends Task<DDLWor
       // drop partitions in the list
       List<Partition> partsToDelete = new ArrayList<Partition>();
       for (Map<String, String> partSpec : dropTbl.getPartSpecs()) {
-        Iterator<Map<String, String>> it = partitions.iterator();
-        while (it.hasNext()) {
-          Map<String, String> part = it.next();
-          // test if partSpec matches part
-          boolean match = true;
-          for (Map.Entry<String, String> item : partSpec.entrySet()) {
-            if (!item.getValue().equals(part.get(item.getKey()))) {
-              match = false;
-              break;
-            }
-          }
-          if (match) {
-            Partition p = db.getPartition(tbl, part, false);
-            if (!p.canDrop()) {
-              throw new HiveException("Table " + tbl.getTableName() +
-                  " Partition " + p.getName() +
-                  " is protected from being dropped");
-            }
-
-            partsToDelete.add(p);
-            it.remove();
+        String partSpecStr;
+        try {
+          partSpecStr = Warehouse.makeSortedPartName(partSpec, false);
+        } catch (MetaException e) {
+          //If the partition spec to remove is empty or null, just move on gracefully
+          LOG.warn(e.getMessage());
+          continue;
+        }
+        Map<String, String> origPart = null;
+        //If the partition we wish to delete exists in our hashtable of all partition names,
+        //get the partition and add it to our list of partitions to delete
+        if ((origPart = partitions.get(partSpecStr)) != null) {
+          Partition p = db.getPartition(tbl, origPart, false);
+          if (!p.canDrop()) {
+            throw new HiveException("Table " + tbl.getTableName() +
+                " Partition " + p.getName() +
+                " is protected from being dropped");
           }
+          partsToDelete.add(p);
+          partitions.remove(partSpecStr);
         }
       }