Skip to content

HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire… #2478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abstractdog merged 1 commit into from
Sep 27, 2021
Merged

HIVE-25331: Create database query doesn't create MANAGEDLOCATION dire… #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...c/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenariosExclusiveReplica.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,18 +700,18 @@ public void testCustomWarehouseLocationsConf() throws Throwable {
private void verifyCustomDBLocations(String srcDb, List<String> listOfTables, String managedCustLocOnSrc,
String externalCustLocOnSrc, boolean replaceCustPath) throws Exception {
Database replDatabase = replica.getDatabase(replicatedDbName);
String managedCustLocOnTgt = new Path(replDatabase.getManagedLocationUri()).toUri().getPath();
String externalCustLocOnTgt = new Path(replDatabase.getLocationUri()).toUri().getPath();
if (replaceCustPath ) {
String managedCustLocOnTgt = new Path(replDatabase.getManagedLocationUri()).toUri().getPath();
Assert.assertEquals(managedCustLocOnSrc, managedCustLocOnTgt);
Assert.assertNotEquals(managedCustLocOnTgt, replica.warehouseRoot.toUri().getPath());
String externalCustLocOnTgt = new Path(replDatabase.getLocationUri()).toUri().getPath();
Assert.assertEquals(externalCustLocOnSrc, externalCustLocOnTgt);
Assert.assertNotEquals(externalCustLocOnTgt, new Path(replica.externalTableWarehouseRoot,
replicatedDbName.toLowerCase() + ".db").toUri().getPath());
} else {
Assert.assertNotEquals(managedCustLocOnSrc, null);
Assert.assertEquals(replDatabase.getManagedLocationUri(), null);
Copy link
Contributor

@abstractdog abstractdog Aug 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkumarsinha: could you please confirm if removing this assertion won't hide any problems with replication?
this patch takes care of setting managed location uri if it's not set by create database

Copy link
Contributor

@pkumarsinha pkumarsinha Sep 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujc714 Rather than removing, I think we should assert that replDatabase.getManagedLocationUri() is in default warehouse location, i.e it should be equal to new Path(replica.warehouseRoot, replicatedDbName.toLowerCase() + ".db")

Copy link
Contributor Author

@ujc714 ujc714 Sep 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pkumarsinha Made the change as you suggested :)

String externalCustLocOnTgt = new Path(replDatabase.getLocationUri()).toUri().getPath();
Assert.assertEquals(managedCustLocOnTgt, new Path(replica.warehouseRoot,
replicatedDbName.toLowerCase() + ".db").toUri().getPath());
Assert.assertNotEquals(externalCustLocOnSrc, externalCustLocOnTgt);
Assert.assertEquals(externalCustLocOnTgt, new Path(replica.externalTableWarehouseRoot,
replicatedDbName.toLowerCase() + ".db").toUri().getPath());
Expand Down
4 changes: 4 additions & 0 deletions .../src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ public void testSimplePrivileges() throws Exception {
db = msc.getDatabase(dbName);
dbLocn = db.getLocationUri();
allowCreateInDb(dbName, userName, dbLocn);
dbLocn = db.getManagedLocationUri();
if (dbLocn != null) {
allowCreateInDb(dbName, userName, dbLocn);
}
tbl.setTableType("EXTERNAL_TABLE");
msc.createTable(tbl);

Expand Down
9 changes: 8 additions & 1 deletion ql/src/java/org/apache/hadoop/hive/ql/ddl/database/create/CreateDatabaseOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,16 @@ private void makeLocationQualified(Database database) throws HiveException {
}

if (database.isSetManagedLocationUri()) {
// TODO should we enforce a location check here?
database.setManagedLocationUri(Utilities.getQualifiedPath(context.getConf(),
new Path(database.getManagedLocationUri())));
} else {
// ManagedLocation is not set we utilize WAREHOUSE together with database name
String rootDir = MetastoreConf.getVar(context.getConf(), MetastoreConf.ConfVars.WAREHOUSE);
Path path = new Path(rootDir, database.getName().toLowerCase() + DATABASE_PATH_SUFFIX);
String qualifiedPath = Utilities.getQualifiedPath(context.getConf(), path);
if (!qualifiedPath.equals(database.getLocationUri())) {
database.setManagedLocationUri(qualifiedPath);
}
}
}
}
4 changes: 4 additions & 0 deletions ql/src/test/queries/clientpositive/create_database.q
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
set hive.support.concurrency=true;
set hive.txn.manager=org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
create database testdb location '/tmp/testdb.db';
create table testdb.test as select 1;
2 changes: 1 addition & 1 deletion ql/src/test/results/clientpositive/llap/alter_change_db_location.q.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ PREHOOK: Input: database:newdb
POSTHOOK: query: describe database extended newDB
POSTHOOK: type: DESCDATABASE
POSTHOOK: Input: database:newdb
newdb location/in/test hive_test_user USER
#### A masked pattern was here ####
Copy link
Contributor

@abstractdog abstractdog Aug 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to double-check in what way the output changed which triggered a q.out masking

Copy link
Contributor Author

@ujc714 ujc714 Aug 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The managedlocation is like "file:/home/robbie/hive/itests/qtest/target/localfs/warehouse/newdb.db" which matches "file:" in QOutProcessor.maskIfContains.

Copy link
Contributor

@abstractdog abstractdog Aug 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I created a followup ticket for this: HIVE-25473 (in order to get useful info back)

Copy link
Contributor Author

@ujc714 ujc714 Aug 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the original output is like:
newdb location/in/test file:/home/robbie/hive/itests/qtest/target/localfs/warehouse/newdb.db hive_test_user USER

The managedlocation is not empty. Because of the pattern "file:/", QOutProcessor masks the whole line.

PREHOOK: query: use newDB
PREHOOK: type: SWITCHDATABASE
PREHOOK: Input: database:newdb
Expand Down
18 changes: 18 additions & 0 deletions ql/src/test/results/clientpositive/llap/create_database.q.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#### A masked pattern was here ####
PREHOOK: type: CREATEDATABASE
PREHOOK: Output: database:testdb
#### A masked pattern was here ####
POSTHOOK: type: CREATEDATABASE
POSTHOOK: Output: database:testdb
#### A masked pattern was here ####
PREHOOK: query: create table testdb.test as select 1
PREHOOK: type: CREATETABLE_AS_SELECT
PREHOOK: Input: _dummy_database@_dummy_table
PREHOOK: Output: database:testdb
PREHOOK: Output: testdb@test
POSTHOOK: query: create table testdb.test as select 1
POSTHOOK: type: CREATETABLE_AS_SELECT
POSTHOOK: Input: _dummy_database@_dummy_table
POSTHOOK: Output: database:testdb
POSTHOOK: Output: testdb@test
POSTHOOK: Lineage: test._c0 SIMPLE []
2 changes: 1 addition & 1 deletion ql/src/test/results/clientpositive/llap/database_location.q.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ PREHOOK: Input: database:db2
POSTHOOK: query: DESCRIBE DATABASE EXTENDED db2
POSTHOOK: type: DESCDATABASE
POSTHOOK: Input: database:db2
db2 database 2 location/in/test hive_test_user USER
#### A masked pattern was here ####
PREHOOK: query: USE db2
PREHOOK: type: SWITCHDATABASE
PREHOOK: Input: database:db2
Expand Down
2 changes: 1 addition & 1 deletion ql/src/test/results/clientpositive/tez/explainanalyze_3.q.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ PREHOOK: Input: database:newdb
POSTHOOK: query: describe database extended newDB
POSTHOOK: type: DESCDATABASE
POSTHOOK: Input: database:newdb
newdb location/in/test hive_test_user USER
newdb location/in/test hdfs://### HDFS PATH ### hive_test_user USER
PREHOOK: query: use newDB
PREHOOK: type: SWITCHDATABASE
PREHOOK: Input: database:newdb
Expand Down
2 changes: 1 addition & 1 deletion ql/src/test/results/clientpositive/tez/explainuser_3.q.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ PREHOOK: Input: database:newdb
POSTHOOK: query: describe database extended newDB
POSTHOOK: type: DESCDATABASE
POSTHOOK: Input: database:newdb
newdb location/in/test hive_test_user USER
newdb location/in/test hdfs://### HDFS PATH ### hive_test_user USER
PREHOOK: query: explain use newDB
PREHOOK: type: SWITCHDATABASE
PREHOOK: Input: database:newdb
Expand Down
2 changes: 1 addition & 1 deletion ...metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1362,7 +1362,7 @@ private void create_database_core(RawStore ms, final Database db)
});
if (madeManagedDir) {
LOG.info("Created database path in managed directory " + dbMgdPath);
} else {
} else if (!isInTest || !isDbReplicationTarget(db)) { // Hive replication tests doesn't drop the db after each test
throw new MetaException(
"Unable to create database managed directory " + dbMgdPath + ", failed to create database " + db.getName());
}
Expand Down