This is an automated email from the ASF dual-hosted git repository.
gsaihemanth pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
The following commit(s) were added to refs/heads/master by this push:
new 0c3b8223564 HIVE-27857: Do not check write permission while dropping
external table or partition (#4860) (Wechar Yu, Reviewed by Ayush Saxena, Sai
Hemanth Gantasala)
0c3b8223564 is described below
commit 0c3b8223564cef7d70d915c855fb8b969517e262
Author: Wechar Yu <[email protected]>
AuthorDate: Tue Jan 9 09:37:42 2024 +0800
HIVE-27857: Do not check write permission while dropping external table or
partition (#4860) (Wechar Yu, Reviewed by Ayush Saxena, Sai Hemanth Gantasala)
---
.../ql/metadata/SessionHiveMetaStoreClient.java | 2 +-
.../apache/hadoop/hive/metastore/HMSHandler.java | 30 ++++++++--------
.../client/TestTablesCreateDropAlterTruncate.java | 40 ++++++++++++++++++++++
3 files changed, 56 insertions(+), 16 deletions(-)
diff --git
a/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
index 31efa27abec..0e3dfb281b4 100644
---
a/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
+++
b/ql/src/java/org/apache/hadoop/hive/ql/metadata/SessionHiveMetaStoreClient.java
@@ -823,7 +823,7 @@ public class SessionHiveMetaStoreClient extends
HiveMetaStoreClientWithLocalCach
if (pathStr != null) {
try {
tablePath = new Path(table.getSd().getLocation());
- if (!getWh().isWritable(tablePath.getParent())) {
+ if (deleteData && !isExternalTable(table) &&
!getWh().isWritable(tablePath.getParent())) {
throw new MetaException("Table metadata not deleted since " +
tablePath.getParent() +
" is not writable by " + SecurityUtils.getUser());
}
diff --git
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
index 7100bf93ae1..54eb8b3c256 100644
---
a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
+++
b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
@@ -2969,7 +2969,7 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
firePreEvent(new PreDropTableEvent(tbl, deleteData, this));
tableDataShouldBeDeleted = checkTableDataShouldBeDeleted(tbl,
deleteData);
- if (tbl.getSd().getLocation() != null) {
+ if (tableDataShouldBeDeleted && tbl.getSd().getLocation() != null) {
tblPath = new Path(tbl.getSd().getLocation());
if (!wh.isWritable(tblPath.getParent())) {
String target = indexName == null ? "Table" : "Index table";
@@ -4971,6 +4971,7 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
Table tbl = null;
Partition part = null;
boolean mustPurge = false;
+ boolean tableDataShouldBeDeleted = false;
long writeId = 0;
Map<String, String> transactionalListenerResponses =
Collections.emptyMap();
@@ -4994,7 +4995,8 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
request.setCatName(catName);
tbl = get_table_core(request);
firePreEvent(new PreDropPartitionEvent(tbl, part, deleteData, this));
-
+
+ tableDataShouldBeDeleted = checkTableDataShouldBeDeleted(tbl,
deleteData);
mustPurge = isMustPurge(envContext, tbl);
writeId = getWriteId(envContext);
@@ -5002,12 +5004,12 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
throw new NoSuchObjectException("Partition doesn't exist. " +
part_vals);
}
isArchived = MetaStoreUtils.isArchived(part);
- if (isArchived) {
+ if (tableDataShouldBeDeleted && isArchived) {
archiveParentDir = MetaStoreUtils.getOriginalLocation(part);
verifyIsWritablePath(archiveParentDir);
}
- if ((part.getSd() != null) && (part.getSd().getLocation() != null)) {
+ if (tableDataShouldBeDeleted && (part.getSd() != null) &&
(part.getSd().getLocation() != null)) {
partPath = new Path(part.getSd().getLocation());
verifyIsWritablePath(partPath);
}
@@ -5027,9 +5029,7 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
} finally {
if (!success) {
ms.rollbackTransaction();
- } else if (checkTableDataShouldBeDeleted(tbl, deleteData) &&
- (partPath != null || archiveParentDir != null)) {
-
+ } else if (tableDataShouldBeDeleted && (partPath != null ||
archiveParentDir != null)) {
LOG.info(mustPurge ?
"dropPartition() will purge " + partPath + " directly, skipping
trash." :
"dropPartition() will move " + partPath + " to trash-directory.");
@@ -5159,7 +5159,7 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
boolean deleteData = request.isSetDeleteData() && request.isDeleteData();
boolean ignoreProtection = request.isSetIgnoreProtection() &&
request.isIgnoreProtection();
boolean needResult = !request.isSetNeedResult() || request.isNeedResult();
-
+
List<PathAndDepth> dirsToDelete = new ArrayList<>();
List<Path> archToDelete = new ArrayList<>();
EnvironmentContext envContext =
@@ -5169,19 +5169,21 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
Table tbl = null;
List<Partition> parts = null;
boolean mustPurge = false;
+ boolean tableDataShouldBeDeleted = false;
long writeId = 0;
Map<String, String> transactionalListenerResponses = null;
boolean needsCm = false;
-
+
try {
ms.openTransaction();
// We need Partition-s for firing events and for result; DN needs
MPartition-s to drop.
// Great... Maybe we could bypass fetching MPartitions by issuing direct
SQL deletes.
tbl = get_table_core(catName, dbName, tblName);
mustPurge = isMustPurge(envContext, tbl);
+ tableDataShouldBeDeleted = checkTableDataShouldBeDeleted(tbl,
deleteData);
writeId = getWriteId(envContext);
-
+
int minCount = 0;
RequestPartsSpec spec = request.getParts();
List<String> partNames = null;
@@ -5245,14 +5247,12 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
if (colNames != null) {
partNames.add(FileUtils.makePartName(colNames, part.getValues()));
}
- // Preserve the old behavior of failing when we cannot write, even w/o
deleteData,
- // and even if the table is external. That might not make any sense.
- if (MetaStoreUtils.isArchived(part)) {
+ if (tableDataShouldBeDeleted && MetaStoreUtils.isArchived(part)) {
Path archiveParentDir = MetaStoreUtils.getOriginalLocation(part);
verifyIsWritablePath(archiveParentDir);
archToDelete.add(archiveParentDir);
}
- if ((part.getSd() != null) && (part.getSd().getLocation() != null)) {
+ if (tableDataShouldBeDeleted && (part.getSd() != null) &&
(part.getSd().getLocation() != null)) {
Path partPath = new Path(part.getSd().getLocation());
verifyIsWritablePath(partPath);
dirsToDelete.add(new PathAndDepth(partPath,
part.getValues().size()));
@@ -5276,7 +5276,7 @@ public class HMSHandler extends FacebookBase implements
IHMSHandler {
} finally {
if (!success) {
ms.rollbackTransaction();
- } else if (checkTableDataShouldBeDeleted(tbl, deleteData)) {
+ } else if (tableDataShouldBeDeleted) {
LOG.info(mustPurge ?
"dropPartition() will purge partition-directories directly,
skipping trash."
: "dropPartition() will move partition-directories to
trash-directory.");
diff --git
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
index 0d38b628abb..cf4fc482808 100644
---
a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
+++
b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
@@ -18,7 +18,10 @@
package org.apache.hadoop.hive.metastore.client;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hive.common.StatsSetupConst;
import org.apache.hadoop.hive.common.TableName;
import org.apache.hadoop.hive.metastore.ColumnType;
@@ -68,6 +71,7 @@ import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import java.io.File;
+import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.ArrayList;
@@ -118,6 +122,7 @@ public class TestTablesCreateDropAlterTruncate extends
MetaStoreClientTest {
extraConf.put("fs.trash.interval", "30"); //
FS_TRASH_INTERVAL_KEY (hadoop-2)
extraConf.put(ConfVars.HIVE_IN_TEST.getVarname(), "true");
extraConf.put(ConfVars.METASTORE_METADATA_TRANSFORMER_CLASS.getVarname(),
" ");
+ extraConf.put(ConfVars.AUTHORIZATION_STORAGE_AUTH_CHECKS.getVarname(),
"true");
startMetaStores(msConf, extraConf);
}
@@ -1563,6 +1568,41 @@ public class TestTablesCreateDropAlterTruncate extends
MetaStoreClientTest {
client.dropTable("nosuch", testTables[0].getDbName(),
testTables[0].getTableName(), true, false);
}
+ @Test(expected = MetaException.class)
+ public void testDropManagedTableWithoutStoragePermission() throws
TException, IOException {
+ String dbName = testTables[0].getDbName();
+ String tblName = testTables[0].getTableName();
+ Table table = client.getTable(dbName, tblName);
+ Path tablePath = new Path(table.getSd().getLocation());
+ FileSystem fs = Warehouse.getFs(tablePath, new Configuration());
+ fs.setPermission(tablePath.getParent(), new FsPermission((short) 0555));
+
+ try {
+ client.dropTable(dbName, tblName);
+ } finally {
+ // recover write permission so that file can be cleaned.
+ fs.setPermission(tablePath.getParent(), new FsPermission((short) 0755));
+ }
+ }
+
+ @Test
+ public void testDropExternalTableWithoutStoragePermission() throws
TException, IOException {
+ // external table
+ String dbName = testTables[4].getDbName();
+ String tblName = testTables[4].getTableName();
+ Table table = client.getTable(dbName, tblName);
+ Path tablePath = new Path(table.getSd().getLocation());
+ FileSystem fs = Warehouse.getFs(tablePath, new Configuration());
+ fs.setPermission(tablePath.getParent(), new FsPermission((short) 0555));
+
+ try {
+ client.dropTable(dbName, tblName);
+ } finally {
+ // recover write permission so that file can be cleaned.
+ fs.setPermission(tablePath.getParent(), new FsPermission((short) 0755));
+ }
+ }
+
/**
* Creates a Table with all of the parameters set. The temporary table is
available only on HS2
* server, so do not use it.
