From dd2dd1398f4e8de6894066f6176b841c6fa760b4 Mon Sep 17 00:00:00 2001 From: rtrivedi Date: Mon, 3 Aug 2026 23:05:01 -0500 Subject: [PATCH 1/2] HIVE-29405: Decouple the alter table from HMSHandler/HiveAlterHandler --- .../hadoop/hive/metastore/HMSHandler.java | 76 +-------- .../hadoop/hive/metastore/IHMSHandler.java | 2 + .../metastore/handler/AlterTableHandler.java | 151 ++++++++++++++++++ .../hive/metastore/handler/BaseHandler.java | 5 + 4 files changed, 164 insertions(+), 70 deletions(-) create mode 100644 standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java 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 76eedeadb617..eb8107e377cc 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 @@ -2076,84 +2076,20 @@ private void alter_partitions_with_environment_context(String catName, String db @Override public AlterTableResponse alter_table_req(AlterTableRequest req) throws InvalidOperationException, MetaException { - alter_table_core(req.getCatName(), req.getDbName(), req.getTableName(), - req.getTable(), req.getEnvironmentContext(), req.getValidWriteIdList(), - req.getProcessorCapabilities(), req.getProcessorIdentifier(), - req.getExpectedParameterKey(), req.getExpectedParameterValue()); - return new AlterTableResponse(); - } - - private void alter_table_core(String catName, String dbname, String name, Table newTable, - EnvironmentContext envContext, String validWriteIdList, List processorCapabilities, - String processorId, String expectedPropertyKey, String expectedPropertyValue) - throws InvalidOperationException, MetaException { - startFunction("alter_table", ": " + TableName.getQualified(catName, dbname, name) - + " newtbl=" + newTable.getTableName()); - if (envContext == null) { - envContext = new EnvironmentContext(); - } - // Set the values to the envContext, so we do not have to change the HiveAlterHandler API - if (expectedPropertyKey != null) { - envContext.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY, expectedPropertyKey); - } - if (expectedPropertyValue != null) { - envContext.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE, expectedPropertyValue); - } - - if (catName == null) { - catName = getDefaultCatalog(conf); - } - - // HIVE-25282: Drop/Alter table in REMOTE db should fail - try { - Database db = get_database_core(catName, dbname); - if (MetaStoreUtils.isDatabaseRemote(db)) { - throw new MetaException("Alter table in REMOTE database " + db.getName() + " is not allowed"); - } - } catch (NoSuchObjectException e) { - throw new InvalidOperationException("Alter table in REMOTE database is not allowed"); - } - - // Update the time if it hasn't been specified. - if (newTable.getParameters() == null || - newTable.getParameters().get(hive_metastoreConstants.DDL_TIME) == null) { - newTable.putToParameters(hive_metastoreConstants.DDL_TIME, Long.toString(System - .currentTimeMillis() / 1000)); - } - - // Adds the missing scheme/authority for the new table location - if (newTable.getSd() != null) { - String newLocation = newTable.getSd().getLocation(); - if (org.apache.commons.lang3.StringUtils.isNotEmpty(newLocation)) { - Path tblPath = wh.getDnsPath(new Path(newLocation)); - newTable.getSd().setLocation(tblPath.toString()); - } - } - // Set the catalog name if it hasn't been set in the new table - if (!newTable.isSetCatName()) { - newTable.setCatName(catName); - } - - boolean success = false; + startFunction("alter_table", ": " + TableName.getQualified(req.getCatName(), req.getDbName(), + req.getTableName()) + " newtbl=" + req.getTable().getTableName()); Exception ex = null; + boolean ret = false; try { - GetTableRequest request = new GetTableRequest(dbname, name); - request.setCatName(catName); - Table oldt = get_table_core(request); - if (transformer != null) { - newTable = transformer.transformAlterTable(oldt, newTable, processorCapabilities, processorId); - } - firePreEvent(new PreAlterTableEvent(oldt, newTable, this)); - alterHandler.alterTable(getMS(), wh, catName, dbname, name, newTable, - envContext, this, validWriteIdList); - success = true; + ret = AbstractRequestHandler.offer(this, req).success(); + return new AlterTableResponse(); } catch (Exception e) { ex = e; throw handleException(e).throwIfInstance(MetaException.class, InvalidOperationException.class) .convertIfInstance(NoSuchObjectException.class, InvalidOperationException.class) .defaultMetaException(); } finally { - endFunction("alter_table", success, ex, name); + endFunction("alter_table", ret, ex, req.getTableName()); } } diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java index 7538ce896bfa..8fde79550360 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java @@ -111,5 +111,7 @@ DataConnector get_dataconnector_core(final String name) IMetaStoreMetadataTransformer getMetadataTransformer(); + AlterHandler getAlterHandler(); + MetaStoreFilterHook getMetaFilterHook(); } diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java new file mode 100644 index 000000000000..77455fae1b8d --- /dev/null +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hive.metastore.handler; + +import java.io.IOException; +import java.util.List; + +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hive.common.TableName; +import org.apache.hadoop.hive.metastore.HMSHandler; +import org.apache.hadoop.hive.metastore.IHMSHandler; +import org.apache.hadoop.hive.metastore.IMetaStoreMetadataTransformer; +import org.apache.hadoop.hive.metastore.api.AlterTableRequest; +import org.apache.hadoop.hive.metastore.api.Database; +import org.apache.hadoop.hive.metastore.api.EnvironmentContext; +import org.apache.hadoop.hive.metastore.api.GetTableRequest; +import org.apache.hadoop.hive.metastore.api.InvalidOperationException; +import org.apache.hadoop.hive.metastore.api.MetaException; +import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants; +import org.apache.hadoop.hive.metastore.events.PreAlterTableEvent; +import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils; +import org.apache.thrift.TException; + +import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog; + +@SuppressWarnings("unused") +@RequestHandler(requestBody = AlterTableRequest.class) +public class AlterTableHandler + extends AbstractRequestHandler { + private String catName; + private String dbname; + private String name; + private Table newTable; + private EnvironmentContext envContext; + private String validWriteIdList; + private List processorCapabilities; + private String processorId; + + AlterTableHandler(IHMSHandler handler, AlterTableRequest request) { + super(handler, false, request); + } + + @Override + protected void beforeExecute() throws TException, IOException { + this.catName = request.isSetCatName() && request.getCatName() != null + ? request.getCatName() : getDefaultCatalog(handler.getConf()); + this.dbname = request.getDbName(); + this.name = request.getTableName(); + this.newTable = request.getTable(); + this.validWriteIdList = request.getValidWriteIdList(); + this.processorCapabilities = request.getProcessorCapabilities(); + this.processorId = request.getProcessorIdentifier(); + + // Build envContext, embedding expected-parameter hints so HiveAlterHandler can read them + // without requiring an API change. + this.envContext = request.getEnvironmentContext() != null + ? request.getEnvironmentContext() : new EnvironmentContext(); + if (request.getExpectedParameterKey() != null) { + envContext.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_KEY, + request.getExpectedParameterKey()); + } + if (request.getExpectedParameterValue() != null) { + envContext.putToProperties(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE, + request.getExpectedParameterValue()); + } + + // HIVE-25282: Drop/Alter table in REMOTE db should fail + try { + Database db = handler.get_database_core(catName, dbname); + if (MetaStoreUtils.isDatabaseRemote(db)) { + throw new MetaException("Alter table in REMOTE database " + db.getName() + " is not allowed"); + } + } catch (NoSuchObjectException e) { + throw new InvalidOperationException("Alter table in REMOTE database is not allowed"); + } + + // Update the time if it hasn't been specified. + if (newTable.getParameters() == null + || newTable.getParameters().get(hive_metastoreConstants.DDL_TIME) == null) { + newTable.putToParameters(hive_metastoreConstants.DDL_TIME, + Long.toString(System.currentTimeMillis() / 1000)); + } + + // Normalise the new table location by adding missing scheme/authority. + if (newTable.getSd() != null) { + String newLocation = newTable.getSd().getLocation(); + if (StringUtils.isNotEmpty(newLocation)) { + Path tblPath = handler.getWh().getDnsPath(new Path(newLocation)); + newTable.getSd().setLocation(tblPath.toString()); + } + } + + // Ensure the catalog name is set on the new table. + if (!newTable.isSetCatName()) { + newTable.setCatName(catName); + } + + // Fetch the current table so we can pass it to the pre-event and transformer. + GetTableRequest getReq = new GetTableRequest(dbname, name); + getReq.setCatName(catName); + Table oldt = handler.get_table_core(getReq); + + IMetaStoreMetadataTransformer transformer = handler.getMetadataTransformer(); + if (transformer != null) { + newTable = transformer.transformAlterTable(oldt, newTable, processorCapabilities, processorId); + } + + ((HMSHandler) handler).firePreEvent(new PreAlterTableEvent(oldt, newTable, handler)); + } + + @Override + protected AlterTableResult execute() throws TException, IOException { + handler.getAlterHandler().alterTable(handler.getMS(), handler.getWh(), + catName, dbname, name, newTable, envContext, handler, validWriteIdList); + return new AlterTableResult(true); + } + + @Override + protected void afterExecute(AlterTableResult result) throws TException, IOException { + // HiveAlterHandler fires both transactional and regular listeners internally. + super.afterExecute(result); + } + + @Override + public String toString() { + return "AlterTableHandler [" + id + "] - alter table " + + TableName.getQualified(catName, dbname, name) + ":"; + } + + public record AlterTableResult(boolean success) implements Result { + } +} diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java index 4664529ccd9a..cb2d30293b6d 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java @@ -172,6 +172,11 @@ public IMetaStoreMetadataTransformer getMetadataTransformer() { return transformer; } + @Override + public AlterHandler getAlterHandler() { + return alterHandler; + } + @Override public MetaStoreFilterHook getMetaFilterHook() { return filterHook; From b3b39ec7df79a936f3be5e87efc8048907d0fb1b Mon Sep 17 00:00:00 2001 From: rtrivedi Date: Thu, 6 Aug 2026 15:27:42 -0500 Subject: [PATCH 2/2] HIVE-29405: moved alterTable() logic from HiveAlterHandler --- .../hadoop/hive/metastore/HMSHandler.java | 4 +- .../hive/metastore/HiveAlterHandler.java | 454 +--------------- .../metastore/handler/AlterTableHandler.java | 485 +++++++++++++++++- .../hive/metastore/TestHiveAlterHandler.java | 15 +- 4 files changed, 490 insertions(+), 468 deletions(-) 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 eb8107e377cc..21afd00a9eb7 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 @@ -2076,7 +2076,9 @@ private void alter_partitions_with_environment_context(String catName, String db @Override public AlterTableResponse alter_table_req(AlterTableRequest req) throws InvalidOperationException, MetaException { - startFunction("alter_table", ": " + TableName.getQualified(req.getCatName(), req.getDbName(), + String catName = req.isSetCatName() && req.getCatName() != null + ? req.getCatName() : getDefaultCatalog(conf); + startFunction("alter_table", ": " + TableName.getQualified(catName, req.getDbName(), req.getTableName()) + " newtbl=" + req.getTable().getTableName()); Exception ex = null; boolean ret = false; diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java index d3ff9e96377d..9dc6f5a063d8 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java @@ -23,12 +23,11 @@ import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hive.common.AcidMetaDataFile.DataFormat; -import org.apache.hadoop.hive.common.repl.ReplConst; import org.apache.hadoop.hive.common.TableName; import org.apache.hadoop.hive.metastore.conf.MetastoreConf; import org.apache.hadoop.hive.metastore.events.AlterPartitionEvent; import org.apache.hadoop.hive.metastore.events.AlterPartitionsEvent; -import org.apache.hadoop.hive.metastore.events.AlterTableEvent; +import org.apache.hadoop.hive.metastore.handler.AlterTableHandler; import org.apache.hadoop.hive.metastore.messaging.EventMessage; import org.apache.hadoop.hive.metastore.model.MTable; import org.apache.hadoop.hive.metastore.utils.FileUtils; @@ -51,7 +50,6 @@ import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; import org.apache.hadoop.hive.metastore.api.Partition; -import org.apache.hadoop.hive.metastore.api.PartitionsRequest; import org.apache.hadoop.hive.metastore.api.Table; import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants; import org.apache.thrift.TException; @@ -73,7 +71,6 @@ import static org.apache.hadoop.hive.metastore.HiveMetaStoreClient.RENAME_PARTITION_MAKE_COPY; import static org.apache.hadoop.hive.metastore.handler.TruncateTableHandler.addTruncateBaseFile; import static org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils.findStaleColumns; -import static org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils.isDbReplicationTarget; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog; import static org.apache.hadoop.hive.metastore.utils.StringUtils.normalizeIdentifier; @@ -105,439 +102,9 @@ public void alterTable(RawStore msdb, Warehouse wh, String catName, String dbnam String name, Table newt, EnvironmentContext environmentContext, IHMSHandler handler, String writeIdList) throws InvalidOperationException, MetaException { - String catalogName = normalizeIdentifier(catName); - String tableName = normalizeIdentifier(name); - String databaseName = normalizeIdentifier(dbname); - - final boolean cascade; - final boolean replDataLocationChanged; - final boolean isReplicated; - if ((environmentContext != null) && environmentContext.isSetProperties()) { - cascade = StatsSetupConst.TRUE.equals(environmentContext.getProperties().get(StatsSetupConst.CASCADE)); - replDataLocationChanged = ReplConst.TRUE.equals(environmentContext.getProperties().get(ReplConst.REPL_DATA_LOCATION_CHANGED)); - } else { - cascade = false; - replDataLocationChanged = false; - } - - if (newt == null) { - throw new InvalidOperationException("New table is null"); - } - - String newTblName = newt.getTableName().toLowerCase(); - String newDbName = newt.getDbName().toLowerCase(); - - if (!MetaStoreUtils.validateName(newTblName, handler.getConf())) { - throw new InvalidOperationException(newTblName + " is not a valid object name"); - } - String validate = MetaStoreServerUtils.validateTblColumns(newt.getSd().getCols()); - if (validate != null) { - throw new InvalidOperationException("Invalid column " + validate); - } - - // Validate bucketedColumns in new table - List bucketColumns = MetaStoreServerUtils.validateBucketColumns(newt.getSd()); - if (CollectionUtils.isNotEmpty(bucketColumns)) { - String errMsg = "Bucket columns - " + bucketColumns + " doesn't match with any table columns"; - LOG.error(errMsg); - throw new InvalidOperationException(errMsg); - } - - Path srcPath = null; - FileSystem srcFs; - Path destPath = null; - FileSystem destFs = null; - - boolean success = false; - boolean dataWasMoved = false; - boolean isPartitionedTable = false; - - Database olddb = null; - Table oldt = null; - - List transactionalListeners = handler.getTransactionalListeners(); - List listeners = handler.getListeners(); - Map txnAlterTableEventResponses = Collections.emptyMap(); - - try { - boolean rename = false; - List parts; - - // Switching tables between catalogs is not allowed. - if (!catalogName.equalsIgnoreCase(newt.getCatName())) { - throw new InvalidOperationException("Tables cannot be moved between catalogs, old catalog" + - catalogName + ", new catalog " + newt.getCatName()); - } - - // check if table with the new name already exists - if (!newTblName.equals(tableName) || !newDbName.equals(databaseName)) { - if (msdb.getTable(catalogName, newDbName, newTblName, null) != null) { - throw new InvalidOperationException("new table " + newDbName - + "." + newTblName + " already exists"); - } - rename = true; - } - - String expectedKey = environmentContext != null && environmentContext.getProperties() != null ? - environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY) : null; - String expectedValue = environmentContext != null && environmentContext.getProperties() != null ? - environmentContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE) : null; - - msdb.openTransaction(); - // get old table - // Note: we don't verify stats here; it's done below in alterTableUpdateTableColumnStats. - olddb = msdb.getDatabase(catalogName, databaseName); - oldt = msdb.getTable(catalogName, databaseName, tableName, null); - if (oldt == null) { - throw new InvalidOperationException("table " + - TableName.getQualified(catalogName, databaseName, tableName) + " doesn't exist"); - } - - if (expectedKey != null && expectedValue != null) { - String newValue = newt.getParameters().get(expectedKey); - if (newValue == null) { - throw new MetaException(String.format("New value for expected key %s is not set", expectedKey)); - } - if (!expectedValue.equals(oldt.getParameters().get(expectedKey))) { - throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is '" - + oldt.getParameters().get(expectedKey) + "'. The expected was value was '" + expectedValue + "'"); - } - long affectedRows = msdb.updateParameterWithExpectedValue(oldt, expectedKey, expectedValue, newValue); - if (affectedRows != 1) { - // make sure concurrent modification exception messages have the same prefix - throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is different"); - } - } - - validateTableChangesOnReplSource(olddb, oldt, newt, environmentContext); - - // On a replica this alter table will be executed only if old and new both the databases are - // available and being replicated into. Otherwise, it will be either create or drop of table. - isReplicated = isDbReplicationTarget(olddb); - if (oldt.getPartitionKeysSize() != 0) { - isPartitionedTable = true; - } - - // Throws InvalidOperationException if the new column types are not - // compatible with the current column types. - DefaultIncompatibleTableChangeHandler.get() - .allowChange(handler.getConf(), oldt, newt); - - //check that partition keys have not changed, except for virtual views - //however, allow the partition comments to change - boolean partKeysPartiallyEqual = checkPartialPartKeysEqual(oldt.getPartitionKeys(), - newt.getPartitionKeys()); - - if (!oldt.getTableType().equals(TableType.VIRTUAL_VIEW.toString())){ - Map properties = environmentContext.getProperties(); - if (properties == null || !Boolean.parseBoolean(properties.getOrDefault(HiveMetaHook.ALLOW_PARTITION_KEY_CHANGE, - "false"))) { - if (!partKeysPartiallyEqual) { - throw new InvalidOperationException("partition keys can not be changed."); - } - } - } - - // Two mutually exclusive flows possible. - // i) Partition locations needs update if replDataLocationChanged is true which means table's - // data location is changed with all partition sub-directories. - // ii) Rename needs change the data location and move the data to the new location corresponding - // to the new name if: - // 1) the table is not a virtual view, and - // 2) the table is not an external table, and - // 3) the user didn't change the default location (or new location is empty), and - // 4) the table was not initially created with a specified location - boolean renamedManagedTable = rename && !oldt.getTableType().equals(TableType.VIRTUAL_VIEW.toString()) - && (oldt.getSd().getLocation().compareTo(newt.getSd().getLocation()) == 0 - || StringUtils.isEmpty(newt.getSd().getLocation())) - && (!MetaStoreUtils.isExternalTable(oldt)); - - Database db = msdb.getDatabase(catalogName, newDbName); - - boolean renamedTranslatedToExternalTable = rename && MetaStoreUtils.isTranslatedToExternalTable(oldt) - && MetaStoreUtils.isTranslatedToExternalTable(newt); - boolean renamedExternalTable = rename && MetaStoreUtils.isExternalTable(oldt) - && !MetaStoreUtils.isPropertyTrue(oldt.getParameters(), HiveMetaHook.TRANSLATED_TO_EXTERNAL); - boolean isRenameIcebergTable = - rename && MetaStoreUtils.isIcebergTable(newt.getParameters()); - - deleteTableColumnStats(msdb, oldt, newt); - - if (!isRenameIcebergTable && - (replDataLocationChanged || renamedManagedTable || renamedTranslatedToExternalTable || - renamedExternalTable)) { - srcPath = new Path(oldt.getSd().getLocation()); - - if (replDataLocationChanged) { - // If data location is changed in replication flow, then new path was already set in - // the newt. Also, it is as good as the data is moved and set dataWasMoved=true so that - // location in partitions are also updated accordingly. - // No need to validate if the destPath exists as in replication flow, data gets replicated - // separately. - destPath = new Path(newt.getSd().getLocation()); - dataWasMoved = true; - } else if (!renamedExternalTable) { - // Rename flow. - // If a table was created in a user specified location using the DDL like - // create table tbl ... location ...., it should be treated like an external table - // in the table rename, its data location should not be changed. We can check - // if the table directory was created directly under its database directory to tell - // if it is such a table - // Same applies to the ACID tables suffixed with the `txnId`, case with `lockless reads`. - String oldtRelativePath = wh.getDatabaseManagedPath(olddb).toUri() - .relativize(srcPath.toUri()).toString(); - boolean tableInSpecifiedLoc = !oldtRelativePath.equalsIgnoreCase(tableName) - && !oldtRelativePath.equalsIgnoreCase(tableName + Path.SEPARATOR); - - - if (renamedTranslatedToExternalTable || !tableInSpecifiedLoc) { - srcFs = wh.getFs(srcPath); - - // get new location - assert(isReplicated == isDbReplicationTarget(db)); - if (renamedTranslatedToExternalTable) { - if (!tableInSpecifiedLoc) { - destPath = new Path(newt.getSd().getLocation()); - } else { - Path databasePath = constructRenamedPath(wh.getDatabaseExternalPath(db), srcPath); - destPath = new Path(databasePath, newTblName); - newt.getSd().setLocation(destPath.toString()); - } - } else { - Path databasePath = constructRenamedPath(wh.getDatabaseManagedPath(db), srcPath); - destPath = new Path(databasePath, newTblName); - newt.getSd().setLocation(destPath.toString()); - } - - destFs = wh.getFs(destPath); - - // check that destination does not exist otherwise we will be - // overwriting data - // check that src and dest are on the same file system - if (!FileUtils.equalsFileSystem(srcFs, destFs)) { - throw new InvalidOperationException("table new location " + destPath - + " is on a different file system than the old location " - + srcPath + ". This operation is not supported"); - } - - try { - if (destFs.exists(destPath)) { - throw new InvalidOperationException("New location for this table " + - TableName.getQualified(catalogName, newDbName, newTblName) + - " already exists : " + destPath); - } - // check that src exists and also checks permissions necessary, rename src to dest - if (srcFs.exists(srcPath) && wh.renameDir(srcPath, destPath, - ReplChangeManager.shouldEnableCm(olddb, oldt))) { - dataWasMoved = true; - } - } catch (IOException | MetaException e) { - LOG.error("Alter Table operation for " + databaseName + "." + tableName + " failed.", e); - throw new InvalidOperationException("Alter Table operation for " + databaseName + "." + tableName + - " failed to move data due to: '" + getSimpleMessage(e) - + "' See hive log file for details."); - } - - if (!HiveMetaStore.isRenameAllowed(olddb, db)) { - LOG.error("Alter Table operation for " + TableName.getQualified(catalogName, databaseName, tableName) + - "to new table = " + TableName.getQualified(catalogName, newDbName, newTblName) + " failed "); - throw new MetaException("Alter table not allowed for table " + - TableName.getQualified(catalogName, databaseName, tableName) + - "to new table = " + TableName.getQualified(catalogName, newDbName, newTblName)); - } - } - } - - if (isPartitionedTable) { - String oldTblLocPath = srcPath.toUri().getPath(); - String newTblLocPath = dataWasMoved ? destPath.toUri().getPath() : null; - - // Do not verify stats parameters on a partitioned table. - msdb.alterTable(catalogName, databaseName, tableName, newt, null); - int partitionBatchSize = MetastoreConf.getIntVar(handler.getConf(), - MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX); - - // alterPartition is only for changing the partition location in the table rename - if (dataWasMoved) { - PartitionsRequest req = new PartitionsRequest(newDbName, newTblName); - req.setCatName(catName); - req.setMaxParts((short) -1); - parts = handler.get_partitions_req(req).getPartitions(); - - for (Partition part : parts) { - String oldPartLoc = part.getSd().getLocation(); - if (oldPartLoc.contains(oldTblLocPath)) { - URI oldUri = new Path(oldPartLoc).toUri(); - String newPath = oldUri.getPath().replace(oldTblLocPath, newTblLocPath); - Path newPartLocPath = new Path(oldUri.getScheme(), oldUri.getAuthority(), newPath); - part.getSd().setLocation(newPartLocPath.toString()); - } - part.setDbName(newDbName); - part.setTableName(newTblName); - } - - Batchable.runBatched(partitionBatchSize, parts, new Batchable() { - @Override - public List run(List input) throws Exception { - msdb.alterPartitions(catalogName, newDbName, newTblName, - input.stream().map(Partition::getValues).collect(Collectors.toList()), - input, newt.getWriteId(), writeIdList); - return Collections.emptyList(); - } - }); - } - Deadline.checkTimeout(); - } else { - msdb.alterTable(catalogName, databaseName, tableName, newt, writeIdList); - } - } else { - // operations other than table rename - if (MetaStoreServerUtils.requireCalStats(null, null, newt, environmentContext) && - !isPartitionedTable) { - assert(isReplicated == isDbReplicationTarget(db)); - // Update table stats. For partitioned table, we update stats in alterPartition() - MetaStoreServerUtils.updateTableStatsSlow(db, newt, wh, false, true, environmentContext); - } - - if (isPartitionedTable) { - //Currently only column related changes can be cascaded in alter table - boolean runPartitionMetadataUpdate = - (cascade && !MetaStoreServerUtils.areSameColumns(oldt.getSd().getCols(), newt.getSd().getCols())); - // we may skip the update entirely if there are only new columns added - runPartitionMetadataUpdate |= - !cascade && !MetaStoreServerUtils.arePrefixColumns(oldt.getSd().getCols(), newt.getSd().getCols()); - - boolean retainOnColRemoval = - MetastoreConf.getBoolVar(handler.getConf(), MetastoreConf.ConfVars.COLSTATS_RETAIN_ON_COLUMN_REMOVAL); - - if (runPartitionMetadataUpdate) { - // Don't validate table-level stats for a partitoned table. - msdb.alterTable(catalogName, databaseName, tableName, newt, null); - - if (cascade || retainOnColRemoval) { - PartitionsRequest req = new PartitionsRequest(dbname, name); - req.setCatName(catName); - req.setMaxParts((short) -1); - parts = handler.get_partitions_req(req).getPartitions(); - Table table = oldt; - int partitionBatchSize = MetastoreConf.getIntVar(handler.getConf(), - MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX); - Map, List>> changedColsToPartNames = new HashMap<>(); - Batchable.runBatched(partitionBatchSize, parts, new Batchable() { - @Override - public List run(List input) throws Exception { - List oldParts = new ArrayList<>(input.size()); - List> partVals = input.stream().map(Partition::getValues).collect(Collectors.toList()); - for (Partition part : input) { - Partition oldPart = new Partition(part); - List oldCols = part.getSd().getCols(); - part.getSd().setCols(newt.getSd().getCols()); - List deletedCols = new ArrayList<>(); - updateOrGetPartitionColumnStats(msdb, catalogName, databaseName, - tableName, part.getValues(), oldCols, table, part, deletedCols); - if (!deletedCols.isEmpty()) { - changedColsToPartNames.compute(deletedCols, (k, v) -> { - if (v == null) v = new ArrayList<>(); - v.add(part.getValues()); - return v; - }); - } - if (!cascade) { - // update changed properties (stats) - oldPart.setParameters(part.getParameters()); - oldParts.add(oldPart); - } - } - Deadline.checkTimeout(); - msdb.alterPartitions(catalogName, databaseName, tableName, - partVals, (cascade) ? input : oldParts, newt.getWriteId(), writeIdList); - return Collections.emptyList(); - } - }); - - for (Map.Entry, List>> entry : changedColsToPartNames.entrySet()) { - List partNames = new ArrayList<>(); - for (List part_vals : entry.getValue()) { - partNames.add(Warehouse.makePartName(table.getPartitionKeys(), part_vals)); - } - msdb.deletePartitionColumnStatistics(catalogName, databaseName, tableName, partNames, entry.getKey(), null); - } - } else { - // clear all column stats to prevent incorract behaviour in case same column is reintroduced - msdb.deleteAllPartitionColumnStatistics( - new TableName(catalogName, databaseName, tableName), writeIdList); - } - } else { - LOG.warn("Alter table not cascaded to partitions."); - msdb.alterTable(catalogName, databaseName, tableName, newt, writeIdList); - } - } else { - msdb.alterTable(catalogName, databaseName, tableName, newt, writeIdList); - } - } - - if (transactionalListeners != null && !transactionalListeners.isEmpty()) { - txnAlterTableEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, - EventMessage.EventType.ALTER_TABLE, - new AlterTableEvent(oldt, newt, false, true, - newt.getWriteId(), handler, isReplicated), - environmentContext); - } - // commit the changes - success = msdb.commitTransaction(); - } catch (InvalidOperationException | MetaException e) { - throw e; - } catch (TException e) { - LOG.debug("Failed to get object from Metastore ", e); - throw new InvalidOperationException( - "Unable to change partition or table." - + " Check metastore logs for detailed stack." + e.getMessage()); - } finally { - if (success) { - // Txn was committed successfully. - // If data location is changed in replication flow, then need to delete the old path. - if (replDataLocationChanged) { - Path deleteOldDataLoc = new Path(oldt.getSd().getLocation()); - boolean isSkipTrash = MetaStoreUtils.isSkipTrash(oldt.getParameters()); - try { - wh.deleteDir(deleteOldDataLoc, isSkipTrash, - ReplChangeManager.shouldEnableCm(olddb, oldt)); - LOG.info("Deleted the old data location: {} for the table: {}", - deleteOldDataLoc, databaseName + "." + tableName); - } catch (MetaException ex) { - // Eat the exception as it doesn't affect the state of existing tables. - // Expect, user to manually drop this path when exception and so logging a warning. - LOG.warn("Unable to delete the old data location: {} for the table: {}", - deleteOldDataLoc, databaseName + "." + tableName); - } - } - } else { - LOG.error("Failed to alter table " + TableName.getQualified(catalogName, databaseName, tableName)); - msdb.rollbackTransaction(); - if (!replDataLocationChanged && dataWasMoved) { - try { - if (destFs.exists(destPath)) { - if (!destFs.rename(destPath, srcPath)) { - LOG.error("Failed to restore data from " + destPath + " to " + srcPath - + " in alter table failure. Manual restore is needed."); - } - } - } catch (IOException e) { - LOG.error("Failed to restore data from " + destPath + " to " + srcPath - + " in alter table failure. Manual restore is needed."); - } - } - } - } - - if (!listeners.isEmpty()) { - // I don't think event notifications in case of failures are necessary, but other HMS operations - // make this call whether the event failed or succeeded. To make this behavior consistent, - // this call is made for failed events also. - MetaStoreListenerNotifier.notifyEvent(listeners, EventMessage.EventType.ALTER_TABLE, - new AlterTableEvent(oldt, newt, false, success, newt.getWriteId(), handler, isReplicated), - environmentContext, txnAlterTableEventResponses, msdb); - } + AlterTableHandler.runDirectAlter(handler, + new AlterTableHandler.DirectAlterContext(msdb, wh, catName, dbname, name, newt, + environmentContext, writeIdList)); } /** @@ -546,7 +113,7 @@ public List run(List input) throws Exception { * @param ex * @return */ - String getSimpleMessage(Exception ex) { + public static String getSimpleMessage(Exception ex) { if(ex instanceof MetaException) { String msg = ex.getMessage(); if(msg == null || !msg.contains("\n")) { @@ -921,9 +488,8 @@ private void blockPartitionLocationChangesOnReplSource(Database db, Table tbl, } // Validate changes to a table to protect against errors on migration during replication. - private void validateTableChangesOnReplSource(Database db, Table oldTbl, Table newTbl, - EnvironmentContext ec) - throws InvalidOperationException { + public static void validateTableChangesOnReplSource(Configuration conf, Database db, Table oldTbl, + Table newTbl, EnvironmentContext ec) throws InvalidOperationException { // If the database is not replication source, nothing to do if (!ReplChangeManager.isSourceOfReplication(db)) { return; @@ -972,7 +538,7 @@ private void validateTableChangesOnReplSource(Database db, Table oldTbl, Table n } } - private boolean checkPartialPartKeysEqual(List oldPartKeys, + public static boolean checkPartialPartKeysEqual(List oldPartKeys, List newPartKeys) { //return true if both are null, or false if one is null and the other isn't if (newPartKeys == null || oldPartKeys == null) { @@ -1002,7 +568,7 @@ private boolean checkPartialPartKeysEqual(List oldPartKeys, * Uses the scheme and authority of the object's current location and the path constructed * using the object's new name to construct a path for the object's new location. */ - private Path constructRenamedPath(Path defaultNewPath, Path currentPath) { + public static Path constructRenamedPath(Path defaultNewPath, Path currentPath) { URI currentUri = currentPath.toUri(); return new Path(currentUri.getScheme(), currentUri.getAuthority(), @@ -1010,7 +576,7 @@ private Path constructRenamedPath(Path defaultNewPath, Path currentPath) { } @VisibleForTesting - public void deleteTableColumnStats(RawStore msdb, Table oldTable, Table newTable) + public static void deleteTableColumnStats(RawStore msdb, Table oldTable, Table newTable) throws InvalidObjectException, MetaException { try { String catName = normalizeIdentifier(oldTable.isSetCatName() diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java index 77455fae1b8d..fe72a9ed4e2a 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AlterTableHandler.java @@ -19,33 +19,68 @@ package org.apache.hadoop.hive.metastore.handler; import java.io.IOException; +import java.net.URI; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hive.common.StatsSetupConst; import org.apache.hadoop.hive.common.TableName; -import org.apache.hadoop.hive.metastore.HMSHandler; +import org.apache.hadoop.hive.common.repl.ReplConst; +import org.apache.hadoop.hive.metastore.Batchable; +import org.apache.hadoop.hive.metastore.Deadline; +import org.apache.hadoop.hive.metastore.HiveAlterHandler; +import org.apache.hadoop.hive.metastore.HiveMetaHook; +import org.apache.hadoop.hive.metastore.HiveMetaStore; import org.apache.hadoop.hive.metastore.IHMSHandler; import org.apache.hadoop.hive.metastore.IMetaStoreMetadataTransformer; +import org.apache.hadoop.hive.metastore.MetaStoreListenerNotifier; +import org.apache.hadoop.hive.metastore.RawStore; +import org.apache.hadoop.hive.metastore.ReplChangeManager; +import org.apache.hadoop.hive.metastore.TableType; +import org.apache.hadoop.hive.metastore.TransactionalMetaStoreEventListener; +import org.apache.hadoop.hive.metastore.Warehouse; import org.apache.hadoop.hive.metastore.api.AlterTableRequest; import org.apache.hadoop.hive.metastore.api.Database; import org.apache.hadoop.hive.metastore.api.EnvironmentContext; +import org.apache.hadoop.hive.metastore.api.FieldSchema; import org.apache.hadoop.hive.metastore.api.GetTableRequest; import org.apache.hadoop.hive.metastore.api.InvalidOperationException; import org.apache.hadoop.hive.metastore.api.MetaException; import org.apache.hadoop.hive.metastore.api.NoSuchObjectException; +import org.apache.hadoop.hive.metastore.api.Partition; +import org.apache.hadoop.hive.metastore.api.PartitionsRequest; import org.apache.hadoop.hive.metastore.api.Table; import org.apache.hadoop.hive.metastore.api.hive_metastoreConstants; +import org.apache.hadoop.hive.metastore.conf.MetastoreConf; +import org.apache.hadoop.hive.metastore.events.AlterTableEvent; import org.apache.hadoop.hive.metastore.events.PreAlterTableEvent; +import org.apache.hadoop.hive.metastore.messaging.EventMessage; +import org.apache.hadoop.hive.metastore.utils.FileUtils; +import org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils; import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils; +import org.apache.hadoop.hive.metastore.DefaultIncompatibleTableChangeHandler; import org.apache.thrift.TException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import static org.apache.hadoop.hive.metastore.utils.MetaStoreServerUtils.isDbReplicationTarget; import static org.apache.hadoop.hive.metastore.utils.MetaStoreUtils.getDefaultCatalog; +import static org.apache.hadoop.hive.metastore.utils.StringUtils.normalizeIdentifier; @SuppressWarnings("unused") @RequestHandler(requestBody = AlterTableRequest.class) public class AlterTableHandler extends AbstractRequestHandler { + private static final Logger LOG = LoggerFactory.getLogger(AlterTableHandler.class); + private String catName; private String dbname; private String name; @@ -55,10 +90,42 @@ public class AlterTableHandler private List processorCapabilities; private String processorId; + private RawStore msdb; + private Warehouse wh; + private Table oldTable; + private boolean isReplicated; + AlterTableHandler(IHMSHandler handler, AlterTableRequest request) { super(handler, false, request); } + AlterTableHandler(IHMSHandler handler, DirectAlterContext ctx) { + super(handler, false, new AlterTableRequest()); + this.msdb = ctx.msdb(); + this.wh = ctx.wh(); + this.catName = ctx.catName(); + this.dbname = ctx.dbname(); + this.name = ctx.name(); + this.newTable = ctx.newTable(); + this.envContext = ctx.envContext(); + this.validWriteIdList = ctx.validWriteIdList(); + } + + public static void runDirectAlter(IHMSHandler handler, DirectAlterContext ctx) + throws InvalidOperationException, MetaException { + try { + AlterTableHandler op = new AlterTableHandler(handler, ctx); + AlterTableResult result = op.alterTableCore(); + op.notifyRegularListeners(result); + } catch (TException e) { + throw new MetaException(e.getMessage()); + } + } + + public record DirectAlterContext(RawStore msdb, Warehouse wh, String catName, String dbname, String name, + Table newTable, EnvironmentContext envContext, String validWriteIdList) { + } + @Override protected void beforeExecute() throws TException, IOException { this.catName = request.isSetCatName() && request.getCatName() != null @@ -70,8 +137,6 @@ protected void beforeExecute() throws TException, IOException { this.processorCapabilities = request.getProcessorCapabilities(); this.processorId = request.getProcessorIdentifier(); - // Build envContext, embedding expected-parameter hints so HiveAlterHandler can read them - // without requiring an API change. this.envContext = request.getEnvironmentContext() != null ? request.getEnvironmentContext() : new EnvironmentContext(); if (request.getExpectedParameterKey() != null) { @@ -83,7 +148,6 @@ protected void beforeExecute() throws TException, IOException { request.getExpectedParameterValue()); } - // HIVE-25282: Drop/Alter table in REMOTE db should fail try { Database db = handler.get_database_core(catName, dbname); if (MetaStoreUtils.isDatabaseRemote(db)) { @@ -93,14 +157,12 @@ protected void beforeExecute() throws TException, IOException { throw new InvalidOperationException("Alter table in REMOTE database is not allowed"); } - // Update the time if it hasn't been specified. if (newTable.getParameters() == null || newTable.getParameters().get(hive_metastoreConstants.DDL_TIME) == null) { newTable.putToParameters(hive_metastoreConstants.DDL_TIME, Long.toString(System.currentTimeMillis() / 1000)); } - // Normalise the new table location by adding missing scheme/authority. if (newTable.getSd() != null) { String newLocation = newTable.getSd().getLocation(); if (StringUtils.isNotEmpty(newLocation)) { @@ -109,12 +171,10 @@ protected void beforeExecute() throws TException, IOException { } } - // Ensure the catalog name is set on the new table. if (!newTable.isSetCatName()) { newTable.setCatName(catName); } - // Fetch the current table so we can pass it to the pre-event and transformer. GetTableRequest getReq = new GetTableRequest(dbname, name); getReq.setCatName(catName); Table oldt = handler.get_table_core(getReq); @@ -124,19 +184,415 @@ protected void beforeExecute() throws TException, IOException { newTable = transformer.transformAlterTable(oldt, newTable, processorCapabilities, processorId); } - ((HMSHandler) handler).firePreEvent(new PreAlterTableEvent(oldt, newTable, handler)); + ((BaseHandler) handler).firePreEvent(new PreAlterTableEvent(oldt, newTable, handler)); } @Override protected AlterTableResult execute() throws TException, IOException { - handler.getAlterHandler().alterTable(handler.getMS(), handler.getWh(), - catName, dbname, name, newTable, envContext, handler, validWriteIdList); - return new AlterTableResult(true); + this.msdb = handler.getMS(); + this.wh = handler.getWh(); + return alterTableCore(); + } + + private AlterTableResult alterTableCore() throws InvalidOperationException, MetaException { + String catalogName = normalizeIdentifier(catName); + String tableName = normalizeIdentifier(name); + String databaseName = normalizeIdentifier(dbname); + final boolean cascade; + final boolean replDataLocationChanged; + if ((envContext != null) && envContext.isSetProperties()) { + cascade = StatsSetupConst.TRUE.equals(envContext.getProperties().get(StatsSetupConst.CASCADE)); + replDataLocationChanged = ReplConst.TRUE.equals(envContext.getProperties().get(ReplConst.REPL_DATA_LOCATION_CHANGED)); + } else { + cascade = false; + replDataLocationChanged = false; + } + if (newTable == null) { + throw new InvalidOperationException("New table is null"); + } + String newTblName = newTable.getTableName().toLowerCase(); + String newDbName = newTable.getDbName().toLowerCase(); + if (!MetaStoreUtils.validateName(newTblName, handler.getConf())) { + throw new InvalidOperationException(newTblName + " is not a valid object name"); + } + String validate = MetaStoreServerUtils.validateTblColumns(newTable.getSd().getCols()); + if (validate != null) { + throw new InvalidOperationException("Invalid column " + validate); + } + // Validate bucketedColumns in new table + List bucketColumns = MetaStoreServerUtils.validateBucketColumns(newTable.getSd()); + if (CollectionUtils.isNotEmpty(bucketColumns)) { + String errMsg = "Bucket columns - " + bucketColumns + " doesn't match with any table columns"; + LOG.error(errMsg); + throw new InvalidOperationException(errMsg); + } + Path srcPath = null; + FileSystem srcFs; + Path destPath = null; + FileSystem destFs = null; + boolean success = false; + boolean dataWasMoved = false; + boolean isPartitionedTable = false; + Database olddb = null; + this.oldTable = null; + List transactionalListeners = handler.getTransactionalListeners(); + + Map txnAlterTableEventResponses = Collections.emptyMap(); + try { + boolean rename = false; + List parts; + // Switching tables between catalogs is not allowed. + if (!catalogName.equalsIgnoreCase(newTable.getCatName())) { + throw new InvalidOperationException("Tables cannot be moved between catalogs, old catalog" + + catalogName + ", new catalog " + newTable.getCatName()); + } + // check if table with the new name already exists + if (!newTblName.equals(tableName) || !newDbName.equals(databaseName)) { + if (msdb.getTable(catalogName, newDbName, newTblName, null) != null) { + throw new InvalidOperationException("new table " + newDbName + + "." + newTblName + " already exists"); + } + rename = true; + } + String expectedKey = envContext != null && envContext.getProperties() != null ? + envContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_KEY) : null; + String expectedValue = envContext != null && envContext.getProperties() != null ? + envContext.getProperties().get(hive_metastoreConstants.EXPECTED_PARAMETER_VALUE) : null; + msdb.openTransaction(); + // get old table + // Note: we don't verify stats here; it's done below in alterTableUpdateTableColumnStats. + olddb = msdb.getDatabase(catalogName, databaseName); + this.oldTable = msdb.getTable(catalogName, databaseName, tableName, null); + if (oldTable == null) { + throw new InvalidOperationException("table " + + TableName.getQualified(catalogName, databaseName, tableName) + " doesn't exist"); + } + if (expectedKey != null && expectedValue != null) { + String newValue = newTable.getParameters().get(expectedKey); + if (newValue == null) { + throw new MetaException(String.format("New value for expected key %s is not set", expectedKey)); + } + if (!expectedValue.equals(oldTable.getParameters().get(expectedKey))) { + throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is '" + + oldTable.getParameters().get(expectedKey) + "'. The expected was value was '" + expectedValue + "'"); + } + long affectedRows = msdb.updateParameterWithExpectedValue(oldTable, expectedKey, expectedValue, newValue); + if (affectedRows != 1) { + // make sure concurrent modification exception messages have the same prefix + throw new MetaException("The table has been modified. The parameter value for key '" + expectedKey + "' is different"); + } + } + HiveAlterHandler.validateTableChangesOnReplSource(handler.getConf(), olddb, oldTable, newTable, + envContext); + // On a replica this alter table will be executed only if old and new both the databases are + // available and being replicated into. Otherwise, it will be either create or drop of table. + this.isReplicated = isDbReplicationTarget(olddb); + if (oldTable.getPartitionKeysSize() != 0) { + isPartitionedTable = true; + } + // Throws InvalidOperationException if the new column types are not + // compatible with the current column types. + DefaultIncompatibleTableChangeHandler.get() + .allowChange(handler.getConf(), oldTable, newTable); + //check that partition keys have not changed, except for virtual views + //however, allow the partition comments to change + boolean partKeysPartiallyEqual = HiveAlterHandler.checkPartialPartKeysEqual( + oldTable.getPartitionKeys(), newTable.getPartitionKeys()); + if (!oldTable.getTableType().equals(TableType.VIRTUAL_VIEW.toString())){ + Map properties = envContext.getProperties(); + if (properties == null || !Boolean.parseBoolean(properties.getOrDefault(HiveMetaHook.ALLOW_PARTITION_KEY_CHANGE, + "false"))) { + if (!partKeysPartiallyEqual) { + throw new InvalidOperationException("partition keys can not be changed."); + } + } + } + // Two mutually exclusive flows possible. + // i) Partition locations needs update if replDataLocationChanged is true which means table's + // data location is changed with all partition sub-directories. + // ii) Rename needs change the data location and move the data to the new location corresponding + // to the new name if: + // 1) the table is not a virtual view, and + // 2) the table is not an external table, and + // 3) the user didn't change the default location (or new location is empty), and + // 4) the table was not initially created with a specified location + boolean renamedManagedTable = rename && !oldTable.getTableType().equals(TableType.VIRTUAL_VIEW.toString()) + && (oldTable.getSd().getLocation().compareTo(newTable.getSd().getLocation()) == 0 + || StringUtils.isEmpty(newTable.getSd().getLocation())) + && (!MetaStoreUtils.isExternalTable(oldTable)); + Database db = msdb.getDatabase(catalogName, newDbName); + boolean renamedTranslatedToExternalTable = rename && MetaStoreUtils.isTranslatedToExternalTable(oldTable) + && MetaStoreUtils.isTranslatedToExternalTable(newTable); + boolean renamedExternalTable = rename && MetaStoreUtils.isExternalTable(oldTable) + && !MetaStoreUtils.isPropertyTrue(oldTable.getParameters(), HiveMetaHook.TRANSLATED_TO_EXTERNAL); + boolean isRenameIcebergTable = + rename && MetaStoreUtils.isIcebergTable(newTable.getParameters()); + HiveAlterHandler.deleteTableColumnStats(msdb, oldTable, newTable); + if (!isRenameIcebergTable && + (replDataLocationChanged || renamedManagedTable || renamedTranslatedToExternalTable || + renamedExternalTable)) { + srcPath = new Path(oldTable.getSd().getLocation()); + if (replDataLocationChanged) { + // If data location is changed in replication flow, then new path was already set in + // the newTable. Also, it is as good as the data is moved and set dataWasMoved=true so that + // location in partitions are also updated accordingly. + // No need to validate if the destPath exists as in replication flow, data gets replicated + // separately. + destPath = new Path(newTable.getSd().getLocation()); + dataWasMoved = true; + } else if (!renamedExternalTable) { + // Rename flow. + // If a table was created in a user specified location using the DDL like + // create table tbl ... location ...., it should be treated like an external table + // in the table rename, its data location should not be changed. We can check + // if the table directory was created directly under its database directory to tell + // if it is such a table + // Same applies to the ACID tables suffixed with the `txnId`, case with `lockless reads`. + String oldtRelativePath = wh.getDatabaseManagedPath(olddb).toUri() + .relativize(srcPath.toUri()).toString(); + boolean tableInSpecifiedLoc = !oldtRelativePath.equalsIgnoreCase(tableName) + && !oldtRelativePath.equalsIgnoreCase(tableName + Path.SEPARATOR); + if (renamedTranslatedToExternalTable || !tableInSpecifiedLoc) { + srcFs = wh.getFs(srcPath); + // get new location + assert(isReplicated == isDbReplicationTarget(db)); + if (renamedTranslatedToExternalTable) { + if (!tableInSpecifiedLoc) { + destPath = new Path(newTable.getSd().getLocation()); + } else { + Path databasePath = HiveAlterHandler.constructRenamedPath( + wh.getDatabaseExternalPath(db), srcPath); + destPath = new Path(databasePath, newTblName); + newTable.getSd().setLocation(destPath.toString()); + } + } else { + Path databasePath = HiveAlterHandler.constructRenamedPath( + wh.getDatabaseManagedPath(db), srcPath); + destPath = new Path(databasePath, newTblName); + newTable.getSd().setLocation(destPath.toString()); + } + destFs = wh.getFs(destPath); + // check that destination does not exist otherwise we will be + // overwriting data + // check that src and dest are on the same file system + if (!FileUtils.equalsFileSystem(srcFs, destFs)) { + throw new InvalidOperationException("table new location " + destPath + + " is on a different file system than the old location " + + srcPath + ". This operation is not supported"); + } + try { + if (destFs.exists(destPath)) { + throw new InvalidOperationException("New location for this table " + + TableName.getQualified(catalogName, newDbName, newTblName) + + " already exists : " + destPath); + } + // check that src exists and also checks permissions necessary, rename src to dest + if (srcFs.exists(srcPath) && wh.renameDir(srcPath, destPath, + ReplChangeManager.shouldEnableCm(olddb, oldTable))) { + dataWasMoved = true; + } + } catch (IOException | MetaException e) { + LOG.error("Alter Table operation for " + databaseName + "." + tableName + " failed.", e); + throw new InvalidOperationException("Alter Table operation for " + databaseName + "." + tableName + + " failed to move data due to: '" + HiveAlterHandler.getSimpleMessage(e) + + "' See hive log file for details."); + } + if (!HiveMetaStore.isRenameAllowed(olddb, db)) { + LOG.error("Alter Table operation for " + TableName.getQualified(catalogName, databaseName, tableName) + + "to new table = " + TableName.getQualified(catalogName, newDbName, newTblName) + " failed "); + throw new MetaException("Alter table not allowed for table " + + TableName.getQualified(catalogName, databaseName, tableName) + + "to new table = " + TableName.getQualified(catalogName, newDbName, newTblName)); + } + } + } + if (isPartitionedTable) { + String oldTblLocPath = srcPath.toUri().getPath(); + String newTblLocPath = dataWasMoved ? destPath.toUri().getPath() : null; + // Do not verify stats parameters on a partitioned table. + msdb.alterTable(catalogName, databaseName, tableName, newTable, null); + int partitionBatchSize = MetastoreConf.getIntVar(handler.getConf(), + MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX); + // alterPartition is only for changing the partition location in the table rename + if (dataWasMoved) { + PartitionsRequest req = new PartitionsRequest(newDbName, newTblName); + req.setCatName(catName); + req.setMaxParts((short) -1); + parts = handler.get_partitions_req(req).getPartitions(); + for (Partition part : parts) { + String oldPartLoc = part.getSd().getLocation(); + if (oldPartLoc.contains(oldTblLocPath)) { + URI oldUri = new Path(oldPartLoc).toUri(); + String newPath = oldUri.getPath().replace(oldTblLocPath, newTblLocPath); + Path newPartLocPath = new Path(oldUri.getScheme(), oldUri.getAuthority(), newPath); + part.getSd().setLocation(newPartLocPath.toString()); + } + part.setDbName(newDbName); + part.setTableName(newTblName); + } + Batchable.runBatched(partitionBatchSize, parts, new Batchable() { + @Override + public List run(List input) throws Exception { + msdb.alterPartitions(catalogName, newDbName, newTblName, + input.stream().map(Partition::getValues).collect(Collectors.toList()), + input, newTable.getWriteId(), validWriteIdList); + return Collections.emptyList(); + } + }); + } + Deadline.checkTimeout(); + } else { + msdb.alterTable(catalogName, databaseName, tableName, newTable, validWriteIdList); + } + } else { + // operations other than table rename + if (MetaStoreServerUtils.requireCalStats(null, null, newTable, envContext) && + !isPartitionedTable) { + assert(isReplicated == isDbReplicationTarget(db)); + // Update table stats. For partitioned table, we update stats in alterPartition() + MetaStoreServerUtils.updateTableStatsSlow(db, newTable, wh, false, true, envContext); + } + if (isPartitionedTable) { + //Currently only column related changes can be cascaded in alter table + boolean runPartitionMetadataUpdate = + (cascade && !MetaStoreServerUtils.areSameColumns(oldTable.getSd().getCols(), newTable.getSd().getCols())); + // we may skip the update entirely if there are only new columns added + runPartitionMetadataUpdate |= + !cascade && !MetaStoreServerUtils.arePrefixColumns(oldTable.getSd().getCols(), newTable.getSd().getCols()); + boolean retainOnColRemoval = + MetastoreConf.getBoolVar(handler.getConf(), MetastoreConf.ConfVars.COLSTATS_RETAIN_ON_COLUMN_REMOVAL); + if (runPartitionMetadataUpdate) { + // Don't validate table-level stats for a partitoned table. + msdb.alterTable(catalogName, databaseName, tableName, newTable, null); + if (cascade || retainOnColRemoval) { + PartitionsRequest req = new PartitionsRequest(dbname, name); + req.setCatName(catName); + req.setMaxParts((short) -1); + parts = handler.get_partitions_req(req).getPartitions(); + Table table = oldTable; + int partitionBatchSize = MetastoreConf.getIntVar(handler.getConf(), + MetastoreConf.ConfVars.BATCH_RETRIEVE_MAX); + Map, List>> changedColsToPartNames = new HashMap<>(); + Batchable.runBatched(partitionBatchSize, parts, new Batchable() { + @Override + public List run(List input) throws Exception { + List oldParts = new ArrayList<>(input.size()); + List> partVals = input.stream().map(Partition::getValues).collect(Collectors.toList()); + for (Partition part : input) { + Partition oldPart = new Partition(part); + List oldCols = part.getSd().getCols(); + part.getSd().setCols(newTable.getSd().getCols()); + List deletedCols = new ArrayList<>(); + HiveAlterHandler.updateOrGetPartitionColumnStats(msdb, catalogName, databaseName, + tableName, part.getValues(), oldCols, table, part, deletedCols); + if (!deletedCols.isEmpty()) { + changedColsToPartNames.compute(deletedCols, (k, v) -> { + if (v == null) v = new ArrayList<>(); + v.add(part.getValues()); + return v; + }); + } + if (!cascade) { + // update changed properties (stats) + oldPart.setParameters(part.getParameters()); + oldParts.add(oldPart); + } + } + Deadline.checkTimeout(); + msdb.alterPartitions(catalogName, databaseName, tableName, + partVals, (cascade) ? input : oldParts, newTable.getWriteId(), validWriteIdList); + return Collections.emptyList(); + } + }); + for (Map.Entry, List>> entry : changedColsToPartNames.entrySet()) { + List partNames = new ArrayList<>(); + for (List part_vals : entry.getValue()) { + partNames.add(Warehouse.makePartName(table.getPartitionKeys(), part_vals)); + } + msdb.deletePartitionColumnStatistics(catalogName, databaseName, tableName, partNames, entry.getKey(), null); + } + } else { + // clear all column stats to prevent incorract behaviour in case same column is reintroduced + msdb.deleteAllPartitionColumnStatistics( + new TableName(catalogName, databaseName, tableName), validWriteIdList); + } + } else { + LOG.warn("Alter table not cascaded to partitions."); + msdb.alterTable(catalogName, databaseName, tableName, newTable, validWriteIdList); + } + } else { + msdb.alterTable(catalogName, databaseName, tableName, newTable, validWriteIdList); + } + } + if (transactionalListeners != null && !transactionalListeners.isEmpty()) { + txnAlterTableEventResponses = MetaStoreListenerNotifier.notifyEvent(transactionalListeners, + EventMessage.EventType.ALTER_TABLE, + new AlterTableEvent(oldTable, newTable, false, true, + newTable.getWriteId(), handler, isReplicated), + envContext); + } + // commit the changes + success = msdb.commitTransaction(); + } catch (InvalidOperationException | MetaException e) { + throw e; + } catch (TException e) { + LOG.debug("Failed to get object from Metastore ", e); + throw new InvalidOperationException( + "Unable to change partition or table." + + " Check metastore logs for detailed stack." + e.getMessage()); + } finally { + if (success) { + // Txn was committed successfully. + // If data location is changed in replication flow, then need to delete the old path. + if (replDataLocationChanged) { + Path deleteOldDataLoc = new Path(oldTable.getSd().getLocation()); + boolean isSkipTrash = MetaStoreUtils.isSkipTrash(oldTable.getParameters()); + try { + wh.deleteDir(deleteOldDataLoc, isSkipTrash, + ReplChangeManager.shouldEnableCm(olddb, oldTable)); + LOG.info("Deleted the old data location: {} for the table: {}", + deleteOldDataLoc, databaseName + "." + tableName); + } catch (MetaException ex) { + // Eat the exception as it doesn't affect the state of existing tables. + // Expect, user to manually drop this path when exception and so logging a warning. + LOG.warn("Unable to delete the old data location: {} for the table: {}", + deleteOldDataLoc, databaseName + "." + tableName); + } + } + } else { + LOG.error("Failed to alter table " + TableName.getQualified(catalogName, databaseName, tableName)); + msdb.rollbackTransaction(); + if (!replDataLocationChanged && dataWasMoved) { + try { + if (destFs.exists(destPath)) { + if (!destFs.rename(destPath, srcPath)) { + LOG.error("Failed to restore data from " + destPath + " to " + srcPath + + " in alter table failure. Manual restore is needed."); + } + } + } catch (IOException e) { + LOG.error("Failed to restore data from " + destPath + " to " + srcPath + + " in alter table failure. Manual restore is needed."); + } + } + } + } + return new AlterTableResult(success, txnAlterTableEventResponses); + } + + private void notifyRegularListeners(AlterTableResult result) throws MetaException, TException { + if (result != null && !handler.getListeners().isEmpty() && newTable != null) { + boolean altered = result.success() && oldTable != null; + MetaStoreListenerNotifier.notifyEvent(handler.getListeners(), EventMessage.EventType.ALTER_TABLE, + new AlterTableEvent(oldTable, newTable, false, altered, + newTable.getWriteId(), handler, isReplicated), + envContext, result.transactionalListenerResponses(), msdb); + } } @Override protected void afterExecute(AlterTableResult result) throws TException, IOException { - // HiveAlterHandler fires both transactional and regular listeners internally. + notifyRegularListeners(result); super.afterExecute(result); } @@ -146,6 +602,7 @@ public String toString() { + TableName.getQualified(catName, dbname, name) + ":"; } - public record AlterTableResult(boolean success) implements Result { + public record AlterTableResult(boolean success, Map transactionalListenerResponses) + implements Result { } } diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java index 40bf21ad19e3..5ce68b40c49b 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveAlterHandler.java @@ -55,13 +55,12 @@ public void testAlterTableAddColNotUpdateStats() throws MetaException, InvalidOb newTable.setSd(newSd); RawStore msdb = Mockito.mock(RawStore.class); + Mockito.when(msdb.getConf()).thenReturn(conf); Mockito.doThrow(new RuntimeException("shouldn't be called")).when(msdb).deleteTableColumnStatistics( Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyList(), Mockito.anyString()); - HiveAlterHandler handler = new HiveAlterHandler(); - handler.setConf(conf); Deadline.registerIfNot(100_000); Deadline.startTimer("updateTableColumnStats"); - handler.deleteTableColumnStats(msdb, oldTable, newTable); + HiveAlterHandler.deleteTableColumnStats(msdb, oldTable, newTable); } @Test @@ -84,11 +83,10 @@ public void testAlterTableDelColUpdateStats() throws Exception { newTable.setSd(newSd); RawStore msdb = Mockito.mock(RawStore.class); - HiveAlterHandler handler = new HiveAlterHandler(); - handler.setConf(conf); + Mockito.when(msdb.getConf()).thenReturn(conf); Deadline.registerIfNot(100_000); Deadline.startTimer("updateTableColumnStats"); - handler.deleteTableColumnStats(msdb, oldTable, newTable); + HiveAlterHandler.deleteTableColumnStats(msdb, oldTable, newTable); Mockito.verify(msdb, Mockito.times(1)).deleteTableColumnStatistics( getDefaultCatalog(conf), oldTable.getDbName(), oldTable.getTableName(), Arrays.asList("col4"), null); } @@ -113,13 +111,12 @@ public void testAlterTableChangePosNotUpdateStats() throws MetaException, Invali newTable.setSd(newSd); RawStore msdb = Mockito.mock(RawStore.class); + Mockito.when(msdb.getConf()).thenReturn(conf); Mockito.doThrow(new RuntimeException("shouldn't be called")).when(msdb).deleteTableColumnStatistics( Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyList(), Mockito.anyString()); - HiveAlterHandler handler = new HiveAlterHandler(); - handler.setConf(conf); Deadline.registerIfNot(100_000); Deadline.startTimer("updateTableColumnStats"); - handler.deleteTableColumnStats(msdb, oldTable, newTable); + HiveAlterHandler.deleteTableColumnStats(msdb, oldTable, newTable); } }