Reworked for comments

Author: Venu Reddy

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CleanupRequest.java

@@ -40,7 +40,7 @@ public CleanupRequest(CleanupRequestBuilder builder) {
     this.location = builder.location;
     this.obsoleteDirs = builder.obsoleteDirs;
     this.purge = builder.purge;
-    this.softDelete = builder.isSoftDelete;
+    this.softDelete = builder.softDelete;
     this.sourceOfReplication = builder.sourceOfReplication;
     this.runAs = builder.runAs;
     this.dbName = builder.dbName;
@@ -86,7 +86,7 @@ public static class CleanupRequestBuilder {
     private String location;
     private List<Path> obsoleteDirs;
     private boolean purge;
-    private boolean isSoftDelete;
+    private boolean softDelete;
     private boolean sourceOfReplication;
     private String runAs;
     private String dbName;
@@ -108,7 +108,7 @@ public CleanupRequestBuilder setPurge(boolean purge) {
     }
 
     public CleanupRequestBuilder setSoftDelete(boolean softDelete) {
-      this.isSoftDelete = softDelete;
+      this.softDelete = softDelete;
       return this;
     }
 

ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/FSRemover.java

@@ -17,10 +17,8 @@
  */
 package org.apache.hadoop.hive.ql.txn.compactor;
 
-import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
-import org.apache.hadoop.hive.common.AcidConstants;
 import org.apache.hadoop.hive.conf.HiveConf;
 import org.apache.hadoop.hive.metastore.ReplChangeManager;
 import org.apache.hadoop.hive.metastore.api.Database;
@@ -138,15 +136,11 @@ private void removeDatabaseDirIfNecessary(Database db, CleanupRequest cr, FileSy
       return;
     }
     Path databasePath = new Path(cr.getLocation()).getParent();
-    for (FileStatus status : fs.listStatus(databasePath)) {
-      if (status.isDirectory() &&
-          status.getPath().getName().matches("(.*)" + AcidConstants.SOFT_DELETE_TABLE_PATTERN)) {
-        return;
+    if (FileUtils.isDirEmpty(fs, databasePath)) {
+      if (cr.isSourceOfReplication()) {
+        replChangeManager.recycle(databasePath, ReplChangeManager.RecycleType.MOVE, cr.isPurge());
       }
+      FileUtils.deleteDir(fs, databasePath, cr.isPurge(), conf);
     }
-    if (cr.isSourceOfReplication()) {
-      replChangeManager.recycle(databasePath, ReplChangeManager.RecycleType.MOVE, cr.isPurge());
-    }
-    FileUtils.deleteDir(fs, databasePath, cr.isPurge(), conf);
   }
 }

ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.hive.ql;
 
 import java.io.File;
+import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -1827,11 +1828,9 @@ private void dropDatabaseCascadeNonBlocking() throws Exception {
 
     runCleaner(hiveConf);
 
-    stat = fs.listStatus(new Path(getWarehouseDir(), database + ".db"),
-      t -> t.getName().matches("(mv_)?" + tableName + "2" + SOFT_DELETE_TABLE_PATTERN));
-    if (stat.length != 0) {
-      Assert.fail("Table data was not removed from FS");
-    }
+    Assert.assertThrows(database + ".db directory should not exist", FileNotFoundException.class, () -> {
+      fs.listStatus(new Path(getWarehouseDir(), database + ".db"));
+    });
   }
 
   @Test

standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/hive_metastoreConstants.java

@@ -1344,4 +1344,9 @@ public static String getHttpPath(String httpPath) {
   public static boolean isDatabaseRemote(Database db) {
     return db != null && db.getType() == DatabaseType.REMOTE;
   }
+
+  /**
+   * Soft delete operation types
+   */
+  public enum SoftDeleteOperation {drop_database, drop_table}
 }

standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java

@@ -40,6 +40,7 @@
 import org.apache.hadoop.hive.metastore.events.DropTableEvent;
 import org.apache.hadoop.hive.metastore.txn.TxnStore;
 import org.apache.hadoop.hive.metastore.txn.TxnUtils;
+import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils;
 
 import java.io.IOException;
 import java.util.Iterator;
@@ -248,11 +249,11 @@ private long getTxnId(EnvironmentContext context) {
   }
 
   private void addSoftDeletePropertiesToRequest(EnvironmentContext context, CompactionRequest request) {
-    if ("true".equalsIgnoreCase(context.getProperties().get(hive_metastoreConstants.SOFT_DELETE_DATABASE))) {
-      request.putToProperties(hive_metastoreConstants.SOFT_DELETE_DATABASE, Boolean.TRUE.toString());
-      if ("true".equalsIgnoreCase(context.getProperties().get(ReplConst.SOURCE_OF_REPLICATION))) {
-        request.putToProperties(ReplConst.SOURCE_OF_REPLICATION, Boolean.TRUE.toString());
-      }
+    request.putToProperties(hive_metastoreConstants.SOFT_DELETE_OPERATION,
+        Optional.ofNullable(context.getProperties().get(hive_metastoreConstants.SOFT_DELETE_OPERATION))
+            .orElse(MetaStoreUtils.SoftDeleteOperation.drop_table.name()));
+    if (Boolean.parseBoolean(context.getProperties().get(ReplConst.SOURCE_OF_REPLICATION))) {
+      request.putToProperties(ReplConst.SOURCE_OF_REPLICATION, Boolean.TRUE.toString());
     }
   }
 }

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AcidEventListener.java

@@ -137,7 +137,8 @@ public DropDatabaseResult execute() throws TException, IOException {
         if (isSoftDelete) {
           context = new EnvironmentContext();
           context.putToProperties(hive_metastoreConstants.TXN_ID, String.valueOf(request.getTxnId()));
-          context.putToProperties(hive_metastoreConstants.SOFT_DELETE_DATABASE, Boolean.TRUE.toString());
+          context.putToProperties(hive_metastoreConstants.SOFT_DELETE_OPERATION,
+              MetaStoreUtils.SoftDeleteOperation.drop_database.name());
           if (ReplChangeManager.isSourceOfReplication(db)) {
             context.putToProperties(ReplConst.SOURCE_OF_REPLICATION, Boolean.TRUE.toString());
           }

standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DropDatabaseHandler.java

@@ -372,10 +372,10 @@ public boolean isAbortedTxnCleanup() {
   }
 
   public boolean isSoftDelete() {
-    return "true".equalsIgnoreCase(getProperty(hive_metastoreConstants.SOFT_DELETE_DATABASE));
+    return getProperty(hive_metastoreConstants.SOFT_DELETE_OPERATION) != null;
   }
 
   public boolean isSourceOfReplication() {
-    return "true".equalsIgnoreCase(getProperty(ReplConst.SOURCE_OF_REPLICATION));
+    return Boolean.parseBoolean(getProperty(ReplConst.SOURCE_OF_REPLICATION));
   }
 }