-
Notifications
You must be signed in to change notification settings - Fork 4.8k
HIVE-29241: Distinguish the default location of the database by catalog #6267
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
|
|
||
| import org.apache.hadoop.hive.metastore.api.AlreadyExistsException; | ||
| import org.apache.hadoop.hive.metastore.api.Catalog; | ||
| import org.apache.hadoop.hive.metastore.conf.MetastoreConf; | ||
| import org.apache.hadoop.hive.ql.ErrorMsg; | ||
| import org.apache.hadoop.hive.ql.ddl.DDLOperation; | ||
| import org.apache.hadoop.hive.ql.ddl.DDLOperationContext; | ||
|
|
@@ -37,7 +38,11 @@ public CreateCatalogOperation(DDLOperationContext context, CreateCatalogDesc des | |
|
|
||
| @Override | ||
| public int execute() throws Exception { | ||
| Catalog catalog = new Catalog(desc.getName(), desc.getLocationUri()); | ||
| String catLocationUri = Optional.ofNullable(desc.getLocationUri()) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. minor. i think
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, i didn't get you here. Could you please describe it in detail? Thank you. |
||
| .orElse(MetastoreConf.getVar(context.getConf(), | ||
| MetastoreConf.ConfVars.WAREHOUSE_CATALOG) + "/" + desc.getName()); | ||
zhangbutao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Catalog catalog = new Catalog(desc.getName(), catLocationUri); | ||
| catalog.setDescription(desc.getComment()); | ||
| Optional.ofNullable(desc.getCatlogProperties()) | ||
| .ifPresent(catalog::setParameters); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,20 +19,21 @@ | |
| package org.apache.hadoop.hive.ql.ddl.database.create; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.Optional; | ||
|
|
||
| import org.apache.commons.lang3.tuple.Pair; | ||
| import org.apache.hadoop.hive.metastore.api.DataConnector; | ||
| import org.apache.hadoop.hive.metastore.api.Database; | ||
| import org.apache.hadoop.hive.metastore.api.DatabaseType; | ||
| import org.apache.hadoop.hive.metastore.api.PrincipalType; | ||
| import org.apache.hadoop.hive.ql.ErrorMsg; | ||
| import org.apache.hadoop.hive.ql.QueryState; | ||
| import org.apache.hadoop.hive.ql.ddl.DDLUtils; | ||
| import org.apache.hadoop.hive.ql.exec.TaskFactory; | ||
| import org.apache.hadoop.hive.ql.ddl.DDLSemanticAnalyzerFactory.DDLType; | ||
| import org.apache.hadoop.hive.ql.ddl.DDLWork; | ||
| import org.apache.hadoop.hive.ql.hooks.ReadEntity; | ||
| import org.apache.hadoop.hive.ql.hooks.WriteEntity; | ||
| import org.apache.hadoop.hive.ql.metadata.HiveUtils; | ||
| import org.apache.hadoop.hive.ql.parse.ASTNode; | ||
| import org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer; | ||
| import org.apache.hadoop.hive.ql.parse.HiveParser; | ||
|
|
@@ -51,10 +52,9 @@ public CreateDatabaseAnalyzer(QueryState queryState) throws SemanticException { | |
| @Override | ||
| public void analyzeInternal(ASTNode root) throws SemanticException { | ||
| Pair<String, String> catDbNamePair = DDLUtils.getCatDbNamePair((ASTNode) root.getChild(0)); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think using a |
||
| String catalogName = catDbNamePair.getLeft(); | ||
| if (catalogName != null && getCatalog(catalogName) == null) { | ||
| throw new SemanticException(ErrorMsg.CATALOG_NOT_EXISTS, catalogName); | ||
| } | ||
| String catalogName = Optional.ofNullable(catDbNamePair.getLeft()) | ||
| .orElse(HiveUtils.getCurrentCatalogOrDefault(conf)); | ||
|
|
||
zhangbutao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| String databaseName = catDbNamePair.getRight(); | ||
|
|
||
| boolean ifNotExists = false; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,7 @@ | |
| import org.apache.hadoop.hive.ql.ddl.DDLOperationContext; | ||
| import org.apache.hadoop.hive.ql.exec.Utilities; | ||
| import org.apache.hadoop.hive.ql.metadata.HiveException; | ||
| import org.apache.hadoop.hive.ql.metadata.HiveUtils; | ||
| import org.apache.hadoop.hive.ql.session.SessionState; | ||
|
|
||
| /** | ||
|
|
@@ -56,12 +57,12 @@ public int execute() throws HiveException { | |
| if (desc.getManagedLocationUri() != null) { | ||
| database.setManagedLocationUri(desc.getManagedLocationUri()); | ||
| } | ||
| makeLocationQualified(database); // TODO catalog. Add catalog prefix for db location. Depend on HIVE-29241. | ||
| makeLocationQualified(database); | ||
| if (database.getLocationUri().equalsIgnoreCase(database.getManagedLocationUri())) { | ||
| throw new HiveException("Managed and external locations for database cannot be the same"); | ||
| } | ||
| } else if (desc.getDatabaseType() == DatabaseType.REMOTE) { | ||
| makeLocationQualified(database); // TODO catalog. Add catalog prefix for db location. Depend on HIVE-29241. | ||
| makeLocationQualified(database); | ||
| database.setConnector_name(desc.getConnectorName()); | ||
| database.setRemote_dbname(desc.getRemoteDbName()); | ||
| } else { // should never be here | ||
|
|
@@ -81,34 +82,62 @@ public int execute() throws HiveException { | |
| } | ||
|
|
||
| private void makeLocationQualified(Database database) throws HiveException { | ||
| String catalogName = database.getCatalogName().toLowerCase(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the names be normalized here: special chars/encoding lowercase?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this stage, the database is being created, and the prerequisite is that a valid catalog already exists — there is no need to double validate the catalog here. |
||
| String dbName = database.getName().toLowerCase(); | ||
| boolean isDefaultCatalog = HiveUtils.isDefaultCatalog(catalogName, context.getConf()); | ||
|
|
||
| // -------- External location -------- | ||
| if (database.isSetLocationUri()) { | ||
| database.setLocationUri(Utilities.getQualifiedPath(context.getConf(), new Path(database.getLocationUri()))); | ||
| } else { | ||
| // Location is not set we utilize WAREHOUSE_EXTERNAL together with database name | ||
| String rootDir = MetastoreConf.getVar(context.getConf(), MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL); | ||
| if (rootDir == null || rootDir.trim().isEmpty()) { | ||
| // Fallback plan | ||
| LOG.warn(String.format( | ||
| "%s is not set, falling back to %s. This could cause external tables to use to managed tablespace.", | ||
| MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL.getVarname(), MetastoreConf.ConfVars.WAREHOUSE.getVarname())); | ||
| 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); | ||
| database.setLocationUri(qualifiedPath); | ||
| String rootDir = getExternalRootDir(isDefaultCatalog); | ||
| Path path = buildDbPath(rootDir, catalogName, dbName, isDefaultCatalog); | ||
| database.setLocationUri(Utilities.getQualifiedPath(context.getConf(), path)); | ||
| } | ||
|
|
||
| // -------- Managed location -------- | ||
| if (database.isSetManagedLocationUri()) { | ||
| 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 rootDir = MetastoreConf.getVar( | ||
| context.getConf(), | ||
| isDefaultCatalog | ||
| ? MetastoreConf.ConfVars.WAREHOUSE | ||
| : MetastoreConf.ConfVars.WAREHOUSE_CATALOG | ||
| ); | ||
|
|
||
| Path path = buildDbPath(rootDir, catalogName, dbName, isDefaultCatalog); | ||
| String qualifiedPath = Utilities.getQualifiedPath(context.getConf(), path); | ||
| if (!qualifiedPath.equals(database.getLocationUri())) { | ||
| database.setManagedLocationUri(qualifiedPath); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private Path buildDbPath(String rootDir, String catalogName, String dbName, boolean isDefaultCatalog) { | ||
| return isDefaultCatalog | ||
| ? new Path(rootDir, dbName + DATABASE_PATH_SUFFIX) | ||
| : new Path(rootDir + "/" + catalogName, dbName + DATABASE_PATH_SUFFIX); | ||
zhangbutao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| private String getExternalRootDir(boolean isDefaultCatalog) { | ||
| MetastoreConf.ConfVars externalVar = isDefaultCatalog | ||
| ? MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL | ||
| : MetastoreConf.ConfVars.WAREHOUSE_CATALOG_EXTERNAL; | ||
|
|
||
| String rootDir = MetastoreConf.getVar(context.getConf(), externalVar); | ||
| if (rootDir != null && !rootDir.trim().isEmpty()) { | ||
| return rootDir; | ||
| } | ||
|
|
||
| MetastoreConf.ConfVars fallbackVar = isDefaultCatalog | ||
| ? MetastoreConf.ConfVars.WAREHOUSE | ||
| : MetastoreConf.ConfVars.WAREHOUSE_CATALOG; | ||
|
|
||
| LOG.warn("{} is not set, falling back to {}. This could cause external tables to use managed tablespace.", | ||
| externalVar.getVarname(), fallbackVar.getVarname()); | ||
|
|
||
| return MetastoreConf.getVar(context.getConf(), fallbackVar); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,7 @@ | |
| import org.apache.hadoop.fs.PathFilter; | ||
| import org.apache.hadoop.hive.common.repl.ReplConst; | ||
| import org.apache.hadoop.hive.conf.HiveConf; | ||
| import org.apache.hadoop.hive.metastore.Warehouse; | ||
|
Check warning on line 30 in ql/src/java/org/apache/hadoop/hive/ql/parse/EximUtil.java
|
||
| import org.apache.hadoop.hive.metastore.api.Database; | ||
| import org.apache.hadoop.hive.metastore.api.FieldSchema; | ||
| import org.apache.hadoop.hive.metastore.conf.MetastoreConf; | ||
|
|
@@ -39,6 +40,7 @@ | |
| import org.apache.hadoop.hive.ql.hooks.WriteEntity; | ||
| import org.apache.hadoop.hive.ql.metadata.Hive; | ||
| import org.apache.hadoop.hive.ql.metadata.HiveException; | ||
| import org.apache.hadoop.hive.ql.metadata.HiveUtils; | ||
| import org.apache.hadoop.hive.ql.metadata.Partition; | ||
| import org.apache.hadoop.hive.ql.metadata.Table; | ||
| import org.apache.hadoop.hive.ql.parse.repl.DumpType; | ||
|
|
@@ -388,14 +390,34 @@ | |
|
|
||
| private static void updateIfCustomDbLocations(Database database, Configuration conf) throws SemanticException { | ||
| try { | ||
| String whLocatoion = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL, | ||
| MetastoreConf.getVar(conf, MetastoreConf.ConfVars.WAREHOUSE)); | ||
| Path dbDerivedLoc = new Path(whLocatoion, database.getName().toLowerCase() + DATABASE_PATH_SUFFIX); | ||
| String catName = database.getCatalogName(); | ||
| String dbName = database.getName().toLowerCase(); | ||
| boolean isDefaultCatalog = HiveUtils.isDefaultCatalog(catName, conf); | ||
|
|
||
| // external warehouse root | ||
| String whLocation = MetastoreConf.getVar(conf, | ||
| isDefaultCatalog ? MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL : MetastoreConf.ConfVars.WAREHOUSE_CATALOG_EXTERNAL, | ||
| MetastoreConf.getVar(conf, | ||
| isDefaultCatalog ? MetastoreConf.ConfVars.WAREHOUSE : MetastoreConf.ConfVars.WAREHOUSE_CATALOG)); | ||
|
|
||
| if (!isDefaultCatalog) { | ||
| whLocation = new Path(whLocation, catName).toString(); | ||
| } | ||
|
|
||
| Path dbDerivedLoc = new Path(whLocation, dbName + DATABASE_PATH_SUFFIX); | ||
| String defaultDbLoc = Utilities.getQualifiedPath((HiveConf) conf, dbDerivedLoc); | ||
| database.putToParameters(ReplConst.REPL_IS_CUSTOM_DB_LOC, | ||
| Boolean.toString(!defaultDbLoc.equals(database.getLocationUri()))); | ||
| String whManagedLocatoion = MetastoreConf.getVar(conf, MetastoreConf.ConfVars.WAREHOUSE); | ||
| Path dbDerivedManagedLoc = new Path(whManagedLocatoion, database.getName().toLowerCase() | ||
|
|
||
| // managed warehouse root | ||
| String whManagedLocatoion = MetastoreConf.getVar(conf, | ||
| isDefaultCatalog ? MetastoreConf.ConfVars.WAREHOUSE | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please see #6267 (comment)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replied in #6267 (comment) |
||
| : MetastoreConf.ConfVars.WAREHOUSE_CATALOG); | ||
|
|
||
| if (!isDefaultCatalog) { | ||
| whManagedLocatoion = new Path(whManagedLocatoion, catName).toString(); | ||
| } | ||
| Path dbDerivedManagedLoc = new Path(whManagedLocatoion, dbName | ||
| + DATABASE_PATH_SUFFIX); | ||
| String defaultDbManagedLoc = Utilities.getQualifiedPath((HiveConf) conf, dbDerivedManagedLoc); | ||
| database.getParameters().put(ReplConst.REPL_IS_CUSTOM_DB_MANAGEDLOC, Boolean.toString( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't the catalog be under the
warehousePath?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. The
warehousePathis the path of the default catalog, under which the paths of Hive databases reside. If the catalog path is placed under thewarehousePath, it will cause confusion with the path information of Hive databases under the default catalog.The path information for the default catalog's databases is controlled by the property
metastore.warehouse.dir, which defaults to/user/hive/warehouse. The path for non-default catalogs is controlled by the new added propertymetastore.warehouse.catalog.dir, which defaults to/user/hive/catalog/warehouse.