This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 5c8c1d99fe [#14039] fix(core): Return consistent conflicts for
metadata renames (#14040)
5c8c1d99fe is described below
commit 5c8c1d99fed331360e87f5f0730251655398c135
Author: mchades <[email protected]>
AuthorDate: Thu Oct 22 11:33:14 2026 +0800
[#14039] fix(core): Return consistent conflicts for metadata renames
(#14040)
### What changes were proposed in this pull request?
Translate metadata-store rename conflicts into the corresponding
`*AlreadyExistsException` for filesets, models, policies, catalogs,
metalakes, and managed tables. Preserve the cause and report the final
target name, including the target schema for table renames.
Document the exception contract and add database-backed regression tests
and REST error-response tests.
### Why are the changes needed?
Renaming to an existing name currently returns HTTP 500 for five of
these types and HTTP 400 for managed tables. These conflicts should
consistently return HTTP 409.
Fix: #14039
Job template conflicts are handled separately in #14018.
### Does this PR introduce _any_ user-facing change?
Yes. Affected rename conflicts return HTTP 409 with the object-specific
exception type and conflicting target name.
### How was this patch tested?
- Relational-store tests cover all six types, repeated renames,
cross-schema table conflicts, and unchanged source/target objects after
a failed rename.
- REST tests verify HTTP 409, the error code, exception type, and
message for all six types.
- 471 tests passed across the affected Core manager, Fileset/Model
operation, and REST test classes. `:api:javadoc` and Spotless checks
also passed.
- MySQL/PostgreSQL Docker tests were not run locally; H2 was used for
relational-store validation.
---
.../org/apache/gravitino/SupportsCatalogs.java | 3 +-
.../org/apache/gravitino/SupportsMetalakes.java | 3 +-
.../org/apache/gravitino/file/FilesetCatalog.java | 3 +-
.../org/apache/gravitino/model/ModelCatalog.java | 3 +-
.../apache/gravitino/policy/PolicyOperations.java | 3 +-
.../org/apache/gravitino/rel/TableCatalog.java | 3 +-
.../catalog/fileset/FilesetCatalogOperations.java | 17 +-
.../fileset/TestFilesetCatalogOperations.java | 39 +++
.../catalog/model/ModelCatalogOperations.java | 15 +-
.../catalog/model/TestModelCatalogOperations.java | 30 ++
.../apache/gravitino/catalog/CatalogManager.java | 14 +-
.../gravitino/catalog/ManagedTableOperations.java | 15 +-
.../apache/gravitino/metalake/MetalakeManager.java | 13 +-
.../org/apache/gravitino/policy/PolicyManager.java | 16 +-
.../java/org/apache/gravitino/tag/TagManager.java | 8 +-
.../org/apache/gravitino/utils/RenameUtils.java | 51 ++++
.../apache/gravitino/policy/TestPolicyManager.java | 27 ++
.../storage/relational/TestRenameConflicts.java | 311 +++++++++++++++++++++
.../org/apache/gravitino/tag/TestTagManager.java | 32 +++
.../apache/gravitino/utils/TestRenameUtils.java | 63 +++++
docs/open-api/models.yaml | 9 +
docs/open-api/policies.yaml | 9 +
.../server/web/rest/TestCatalogOperations.java | 21 ++
.../server/web/rest/TestFilesetOperations.java | 22 ++
.../server/web/rest/TestMetalakeOperations.java | 24 ++
.../server/web/rest/TestModelOperations.java | 22 ++
.../server/web/rest/TestPolicyOperations.java | 22 ++
.../server/web/rest/TestTableOperations.java | 22 ++
28 files changed, 787 insertions(+), 33 deletions(-)
diff --git a/api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
b/api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
index f79ace08ed..86584000aa 100644
--- a/api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
+++ b/api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
@@ -172,10 +172,11 @@ public interface SupportsCatalogs {
* @param changes the changes to apply to the catalog.
* @return The altered catalog.
* @throws NoSuchCatalogException If the catalog does not exist.
+ * @throws CatalogAlreadyExistsException If the new name is already used by
another catalog.
* @throws IllegalArgumentException If the changes cannot be applied to the
catalog.
*/
Catalog alterCatalog(String catalogName, CatalogChange... changes)
- throws NoSuchCatalogException, IllegalArgumentException;
+ throws NoSuchCatalogException, CatalogAlreadyExistsException,
IllegalArgumentException;
/**
* Drop a catalog with specified name. Please make sure:
diff --git a/api/src/main/java/org/apache/gravitino/SupportsMetalakes.java
b/api/src/main/java/org/apache/gravitino/SupportsMetalakes.java
index 515b2d836c..31a382ab8f 100644
--- a/api/src/main/java/org/apache/gravitino/SupportsMetalakes.java
+++ b/api/src/main/java/org/apache/gravitino/SupportsMetalakes.java
@@ -83,10 +83,11 @@ public interface SupportsMetalakes {
* @param changes The changes to apply.
* @return The altered metalake.
* @throws NoSuchMetalakeException If the metalake does not exist.
+ * @throws MetalakeAlreadyExistsException If the new name is already used by
another metalake.
* @throws IllegalArgumentException If the changes cannot be applied to the
metalake.
*/
Metalake alterMetalake(String name, MetalakeChange... changes)
- throws NoSuchMetalakeException, IllegalArgumentException;
+ throws NoSuchMetalakeException, MetalakeAlreadyExistsException,
IllegalArgumentException;
/**
* Drop a metalake with specified name. Please make sure:
diff --git a/api/src/main/java/org/apache/gravitino/file/FilesetCatalog.java
b/api/src/main/java/org/apache/gravitino/file/FilesetCatalog.java
index 5f258f384e..93d79f49d6 100644
--- a/api/src/main/java/org/apache/gravitino/file/FilesetCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/file/FilesetCatalog.java
@@ -215,10 +215,11 @@ public interface FilesetCatalog {
* @param changes The changes to apply to the fileset.
* @return The altered fileset metadata.
* @throws NoSuchFilesetException If the fileset does not exist.
+ * @throws FilesetAlreadyExistsException If the new name is already used by
another fileset.
* @throws IllegalArgumentException If the change is rejected by the
implementation.
*/
Fileset alterFileset(NameIdentifier ident, FilesetChange... changes)
- throws NoSuchFilesetException, IllegalArgumentException;
+ throws NoSuchFilesetException, FilesetAlreadyExistsException,
IllegalArgumentException;
/**
* Drop a fileset from the catalog.
diff --git a/api/src/main/java/org/apache/gravitino/model/ModelCatalog.java
b/api/src/main/java/org/apache/gravitino/model/ModelCatalog.java
index b890ddd04b..56d560373c 100644
--- a/api/src/main/java/org/apache/gravitino/model/ModelCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/model/ModelCatalog.java
@@ -343,10 +343,11 @@ public interface ModelCatalog {
* @param changes the several {@link ModelChange} instances to apply to the
model
* @return the updated {@link Model} instance
* @throws NoSuchModelException If the model does not exist
+ * @throws ModelAlreadyExistsException If the new name is already used by
another model.
* @throws IllegalArgumentException If the change is rejected by the
implementation
*/
Model alterModel(NameIdentifier ident, ModelChange... changes)
- throws NoSuchModelException, IllegalArgumentException;
+ throws NoSuchModelException, ModelAlreadyExistsException,
IllegalArgumentException;
/**
* Applies the specified {@link ModelVersionChange changes} to a model
version identified by its
diff --git
a/api/src/main/java/org/apache/gravitino/policy/PolicyOperations.java
b/api/src/main/java/org/apache/gravitino/policy/PolicyOperations.java
index 9a52b2cf57..30c0e24235 100644
--- a/api/src/main/java/org/apache/gravitino/policy/PolicyOperations.java
+++ b/api/src/main/java/org/apache/gravitino/policy/PolicyOperations.java
@@ -94,10 +94,11 @@ public interface PolicyOperations {
* @param changes The changes to apply to the policy.
* @return The altered policy.
* @throws NoSuchPolicyException If the policy does not exist.
+ * @throws PolicyAlreadyExistsException If the new name is already used by
another policy.
* @throws IllegalArgumentException If the changes cannot be associated with
the policy.
*/
Policy alterPolicy(String name, PolicyChange... changes)
- throws NoSuchPolicyException, IllegalArgumentException;
+ throws NoSuchPolicyException, PolicyAlreadyExistsException,
IllegalArgumentException;
/**
* Delete a policy under a metalake.
diff --git a/api/src/main/java/org/apache/gravitino/rel/TableCatalog.java
b/api/src/main/java/org/apache/gravitino/rel/TableCatalog.java
index 6f564760f3..b20e1676fc 100644
--- a/api/src/main/java/org/apache/gravitino/rel/TableCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/rel/TableCatalog.java
@@ -288,10 +288,11 @@ public interface TableCatalog {
* @param changes Table changes to apply to the table.
* @return The updated table metadata.
* @throws NoSuchTableException If the table does not exist.
+ * @throws TableAlreadyExistsException If the new name is already used by
another table.
* @throws IllegalArgumentException If the change is rejected by the
implementation.
*/
Table alterTable(NameIdentifier ident, TableChange... changes)
- throws NoSuchTableException, IllegalArgumentException;
+ throws NoSuchTableException, TableAlreadyExistsException,
IllegalArgumentException;
/**
* Removes both the metadata and the directory associated with the table
from the file system if
diff --git
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
index f1de5114b1..e00f57a26e 100644
---
a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java
@@ -99,7 +99,6 @@ import org.apache.gravitino.credential.CredentialConstants;
import org.apache.gravitino.credential.CredentialUtils;
import org.apache.gravitino.credential.PathBasedCredentialContext;
import org.apache.gravitino.dto.file.FileInfoDTO;
-import org.apache.gravitino.exceptions.AlreadyExistsException;
import org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
@@ -128,6 +127,7 @@ import org.apache.gravitino.utils.FilesetUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.gravitino.utils.RenameUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
@@ -686,7 +686,7 @@ public class FilesetCatalogOperations extends
ManagedSchemaOperations
@Override
public Fileset alterFileset(NameIdentifier ident, FilesetChange... changes)
- throws NoSuchFilesetException, IllegalArgumentException {
+ throws NoSuchFilesetException, FilesetAlreadyExistsException,
IllegalArgumentException {
try {
if (!store.exists(ident, Entity.EntityType.FILESET)) {
throw new NoSuchFilesetException(FILESET_DOES_NOT_EXIST_MSG, ident);
@@ -727,10 +727,15 @@ public class FilesetCatalogOperations extends
ManagedSchemaOperations
throw ExceptionMessages.wrap("Failed to update fileset " + ident, ioe);
} catch (NoSuchEntityException nsee) {
throw new NoSuchFilesetException(nsee, FILESET_DOES_NOT_EXIST_MSG,
ident);
- } catch (AlreadyExistsException aee) {
- // This is happened when renaming a fileset to an existing fileset name.
- throw ExceptionMessages.wrap(
- "Fileset with the same name " + ident.name() + " already exists",
aee);
+ } catch (EntityAlreadyExistsException e) {
+ String newName =
+ RenameUtils.getLastRenameName(
+ ident.name(),
+ changes,
+ FilesetChange.RenameFileset.class,
+ FilesetChange.RenameFileset::getNewName);
+ throw new FilesetAlreadyExistsException(
+ e, "Fileset %s already exists", NameIdentifier.of(ident.namespace(),
newName));
} finally {
if (!alterCommitted) {
secretManager.rollbackSecrets(writtenSecretMaterials.get());
diff --git
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
index f0d5030965..fd46f36805 100644
---
a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
+++
b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java
@@ -90,6 +90,7 @@ import org.apache.gravitino.Catalog;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
@@ -1321,6 +1322,44 @@ public class TestFilesetCatalogOperations {
}
}
+ @ParameterizedTest
+ @EnumSource(Fileset.Type.class)
+ void testRenameFilesetToExistingName(Fileset.Type type) throws IOException {
+ String schemaName = "rename_conflict_" + generateTestId();
+ String schemaPath = TEST_ROOT_PATH + "/" + schemaName;
+ createSchema(schemaName, "schema", null, schemaPath);
+ Fileset source =
+ createFileset("source", schemaName, "source comment", type, null,
schemaPath + "/source");
+ Fileset target =
+ createFileset("target", schemaName, "target comment", type, null,
schemaPath + "/target");
+ NameIdentifier sourceIdent = NameIdentifier.of("m1", "c1", schemaName,
"source");
+ NameIdentifier targetIdent = NameIdentifier.of("m1", "c1", schemaName,
"target");
+
+ try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store,
secretManager)) {
+ ops.initialize(Maps.newHashMap(), randomCatalogInfo("m1", "c1"),
FILESET_PROPERTIES_METADATA);
+ for (FilesetChange[] changes :
+ new FilesetChange[][] {
+ {FilesetChange.rename("target")},
+ {FilesetChange.rename("intermediate"),
FilesetChange.rename("target")}
+ }) {
+ FilesetAlreadyExistsException exception =
+ Assertions.assertThrows(
+ FilesetAlreadyExistsException.class, () ->
ops.alterFileset(sourceIdent, changes));
+ Assertions.assertEquals(
+ "Fileset " + targetIdent + " already exists",
exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ Assertions.assertEquals(source.comment(),
ops.loadFileset(sourceIdent).comment());
+ Assertions.assertEquals(
+ source.storageLocations(),
ops.loadFileset(sourceIdent).storageLocations());
+ Assertions.assertEquals(target.comment(),
ops.loadFileset(targetIdent).comment());
+ Assertions.assertEquals(
+ target.storageLocations(),
ops.loadFileset(targetIdent).storageLocations());
+ Assertions.assertFalse(
+ ops.filesetExists(NameIdentifier.of(sourceIdent.namespace(),
"intermediate")));
+ }
+ }
+ }
+
@Test
public void testAlterFilesetProperties() throws IOException {
final long testId = generateTestId();
diff --git
a/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
b/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
index 1d0c232366..86dfa19d21 100644
---
a/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
+++
b/catalogs/catalog-model/src/main/java/org/apache/gravitino/catalog/model/ModelCatalogOperations.java
@@ -58,6 +58,7 @@ import org.apache.gravitino.utils.ExceptionMessages;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.gravitino.utils.RenameUtils;
public class ModelCatalogOperations extends ManagedSchemaOperations
implements CatalogOperations, ModelCatalog {
@@ -301,7 +302,7 @@ public class ModelCatalogOperations extends
ManagedSchemaOperations
/** {@inheritDoc} */
@Override
public Model alterModel(NameIdentifier ident, ModelChange... changes)
- throws NoSuchModelException, IllegalArgumentException {
+ throws NoSuchModelException, ModelAlreadyExistsException,
IllegalArgumentException {
try {
if (!store.exists(ident, Entity.EntityType.MODEL)) {
throw new NoSuchModelException("Model %s does not exist", ident);
@@ -324,9 +325,15 @@ public class ModelCatalogOperations extends
ManagedSchemaOperations
throw ExceptionMessages.wrap("Failed to load model " + ident, ioe);
} catch (NoSuchEntityException nsee) {
throw new NoSuchModelException(nsee, "Model %s does not exist", ident);
- } catch (EntityAlreadyExistsException eaee) {
- // This is happened when renaming a model to an existing model name.
- throw ExceptionMessages.wrap("Model already exist " + ident.name(),
eaee);
+ } catch (EntityAlreadyExistsException e) {
+ String newName =
+ RenameUtils.getLastRenameName(
+ ident.name(),
+ changes,
+ ModelChange.RenameModel.class,
+ ModelChange.RenameModel::newName);
+ throw new ModelAlreadyExistsException(
+ e, "Model %s already exists", NameIdentifier.of(ident.namespace(),
newName));
}
}
diff --git
a/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
b/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
index 510f9b9e4a..7d7c02a7cd 100644
---
a/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
+++
b/catalogs/catalog-model/src/test/java/org/apache/gravtitino/catalog/model/TestModelCatalogOperations.java
@@ -63,6 +63,7 @@ import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
import org.apache.gravitino.GravitinoEnv;
@@ -961,6 +962,35 @@ public class TestModelCatalogOperations {
Assertions.assertEquals(properties, alteredModel.properties());
}
+ @Test
+ void testRenameModelToExistingName() {
+ String schemaName = randomSchemaName();
+ createSchema(schemaName);
+ NameIdentifier source =
+ NameIdentifierUtil.ofModel(METALAKE_NAME, CATALOG_NAME, schemaName,
"source");
+ NameIdentifier target = NameIdentifier.of(source.namespace(), "target");
+ ops.registerModel(
+ source, "source comment",
StringIdentifier.newPropertiesWithId(newStringId(), null));
+ ops.registerModel(
+ target, "target comment",
StringIdentifier.newPropertiesWithId(newStringId(), null));
+
+ for (ModelChange[] changes :
+ new ModelChange[][] {
+ {ModelChange.rename("target")},
+ {ModelChange.rename("intermediate"), ModelChange.rename("target")}
+ }) {
+ ModelAlreadyExistsException exception =
+ Assertions.assertThrows(
+ ModelAlreadyExistsException.class, () -> ops.alterModel(source,
changes));
+ Assertions.assertEquals("Model " + target + " already exists",
exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ Assertions.assertEquals("source comment",
ops.getModel(source).comment());
+ Assertions.assertEquals("target comment",
ops.getModel(target).comment());
+ Assertions.assertFalse(
+ ops.modelExists(NameIdentifier.of(source.namespace(),
"intermediate")));
+ }
+ }
+
@Test
void testAddModelProperty() {
String schemaName = randomSchemaName();
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
index 206d80a970..8caaf632e4 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -128,6 +128,7 @@ import org.apache.gravitino.utils.IsolatedClassLoader;
import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PooledClassLoaderEntry;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.gravitino.utils.RenameUtils;
import org.apache.gravitino.utils.ThrowableFunction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -1158,11 +1159,12 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
* @param changes The changes to apply to the catalog.
* @return A metadata snapshot of the altered catalog.
* @throws NoSuchCatalogException If the specified catalog does not exist.
+ * @throws CatalogAlreadyExistsException If the new name is already used by
another catalog.
* @throws IllegalArgumentException If an unsupported catalog change is
provided.
*/
@Override
public Catalog alterCatalog(NameIdentifier ident, CatalogChange... changes)
- throws NoSuchCatalogException, IllegalArgumentException {
+ throws NoSuchCatalogException, CatalogAlreadyExistsException,
IllegalArgumentException {
TreeLockUtils.doWithTreeLock(
ident,
@@ -1245,6 +1247,16 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
LOG.warn("Catalog {} does not exist", ident, ne);
throw new NoSuchCatalogException(CATALOG_DOES_NOT_EXIST_MSG,
ident);
+ } catch (EntityAlreadyExistsException e) {
+ String newName =
+ RenameUtils.getLastRenameName(
+ ident.name(),
+ changes,
+ CatalogChange.RenameCatalog.class,
+ CatalogChange.RenameCatalog::getNewName);
+ throw new CatalogAlreadyExistsException(
+ e, "Catalog %s already exists",
NameIdentifier.of(ident.namespace(), newName));
+
} catch (IOException ioe) {
LOG.error("Failed to alter catalog {}", ident, ioe);
throw new RuntimeException(ioe);
diff --git
a/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
b/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
index 64c9af8b9c..e93c8f2cee 100644
---
a/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
+++
b/core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java
@@ -30,6 +30,7 @@ import java.util.Comparator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import org.apache.gravitino.Entity;
@@ -164,10 +165,11 @@ public abstract class ManagedTableOperations implements
TableCatalog {
@Override
public Table alterTable(NameIdentifier ident, TableChange... changes)
- throws NoSuchTableException, IllegalArgumentException {
+ throws NoSuchTableException, TableAlreadyExistsException,
IllegalArgumentException {
// The alterTable in ManagedTableOperations only updates the table
metadata in the entity store.
// It doesn't handle any additional operations like modifying physical
data, etc. Those
// operations should be handled in the specific catalog implementation.
+ AtomicReference<NameIdentifier> targetIdent = new AtomicReference<>(ident);
try {
TableEntity newTableEntity =
store()
@@ -175,14 +177,17 @@ public abstract class ManagedTableOperations implements
TableCatalog {
ident,
TableEntity.class,
Entity.EntityType.TABLE,
- oldEntity -> applyChanges(oldEntity, changes));
+ oldEntity -> {
+ TableEntity updatedEntity = applyChanges(oldEntity,
changes);
+ targetIdent.set(updatedEntity.nameIdentifier());
+ return updatedEntity;
+ });
return toGenericTable(newTableEntity);
} catch (NoSuchEntityException e) {
throw new NoSuchTableException(e, "Table %s does not exist", ident);
} catch (EntityAlreadyExistsException e) {
- throw new IllegalArgumentException(
- "Failed to rename table " + ident + " due to table already exists:
", e);
+ throw new TableAlreadyExistsException(e, "Table %s already exists",
targetIdent.get());
} catch (IOException e) {
throw new RuntimeException("Failed to alter table " + ident, e);
}
@@ -233,7 +238,7 @@ public abstract class ManagedTableOperations implements
TableCatalog {
rename
.getNewSchemaName()
.map(s -> NamespaceUtil.ofTable(oldNs.level(0),
oldNs.level(1), s))
- .orElse(oldNs);
+ .orElse(newNs);
} else if (change instanceof TableChange.UpdateComment updateComment) {
newComment = updateComment.getNewComment();
diff --git
a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java
b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java
index 961906bee0..59230a6a97 100644
--- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java
+++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java
@@ -58,6 +58,7 @@ import org.apache.gravitino.storage.IdGenerator;
import org.apache.gravitino.utils.Executable;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.gravitino.utils.RenameUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -305,12 +306,13 @@ public class MetalakeManager implements
MetalakeDispatcher, Closeable {
* @param changes The array of MetalakeChange objects representing the
changes to apply.
* @return The altered Metalake instance after applying the changes.
* @throws NoSuchMetalakeException If the Metalake with the given identifier
does not exist.
+ * @throws MetalakeAlreadyExistsException If the new name is already used by
another metalake.
* @throws IllegalArgumentException If the provided changes are invalid.
* @throws RuntimeException If altering the Metalake encounters storage
issues.
*/
@Override
public BaseMetalake alterMetalake(NameIdentifier ident, MetalakeChange...
changes)
- throws NoSuchMetalakeException, IllegalArgumentException {
+ throws NoSuchMetalakeException, MetalakeAlreadyExistsException,
IllegalArgumentException {
Executable<BaseMetalake, RuntimeException> exceptionExecutable =
() -> {
try {
@@ -341,6 +343,15 @@ public class MetalakeManager implements
MetalakeDispatcher, Closeable {
LOG.warn("Altering Metalake {} failed due to invalid changes",
ident, iae);
throw iae;
+ } catch (EntityAlreadyExistsException e) {
+ String newName =
+ RenameUtils.getLastRenameName(
+ ident.name(),
+ changes,
+ MetalakeChange.RenameMetalake.class,
+ MetalakeChange.RenameMetalake::getNewName);
+ throw new MetalakeAlreadyExistsException(e, "Metalake %s already
exists", newName);
+
} catch (IOException ioe) {
LOG.error("Loading Metalake {} failed due to storage issues",
ident, ioe);
throw new RuntimeException(ioe);
diff --git a/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
b/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
index db194dd0b3..eb3c8b5848 100644
--- a/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
+++ b/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
@@ -45,6 +45,7 @@ import org.apache.gravitino.utils.MetadataObjectUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.gravitino.utils.RenameUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -148,7 +149,8 @@ public class PolicyManager implements PolicyDispatcher {
}
@Override
- public PolicyEntity alterPolicy(String metalake, String policyName,
PolicyChange... changes) {
+ public PolicyEntity alterPolicy(String metalake, String policyName,
PolicyChange... changes)
+ throws NoSuchPolicyException, PolicyAlreadyExistsException {
NameIdentifier metalakeIdent = NameIdentifierUtil.ofMetalake(metalake);
checkMetalake(metalakeIdent, entityStore);
try {
@@ -161,10 +163,14 @@ public class PolicyManager implements PolicyDispatcher {
throw new NoSuchPolicyException(
"Policy with name %s under metalake %s does not exist", policyName,
metalake);
} catch (EntityAlreadyExistsException e) {
- throw new RuntimeException(
- String.format(
- "Trying to alter policy %s under metalake %s, but the new name
already exists",
- policyName, metalake));
+ String newName =
+ RenameUtils.getLastRenameName(
+ policyName,
+ changes,
+ PolicyChange.RenamePolicy.class,
+ PolicyChange.RenamePolicy::getNewName);
+ throw new PolicyAlreadyExistsException(
+ e, "Policy with name %s under metalake %s already exists", newName,
metalake);
} catch (IOException ioe) {
LOG.error("Failed to alter policy {} under metalake {}", policyName,
metalake, ioe);
throw new RuntimeException(ioe);
diff --git a/core/src/main/java/org/apache/gravitino/tag/TagManager.java
b/core/src/main/java/org/apache/gravitino/tag/TagManager.java
index b35731eed4..63611c1f7c 100644
--- a/core/src/main/java/org/apache/gravitino/tag/TagManager.java
+++ b/core/src/main/java/org/apache/gravitino/tag/TagManager.java
@@ -61,6 +61,7 @@ import org.apache.gravitino.utils.MetadataObjectUtil;
import org.apache.gravitino.utils.NameIdentifierUtil;
import org.apache.gravitino.utils.NamespaceUtil;
import org.apache.gravitino.utils.PrincipalUtils;
+import org.apache.gravitino.utils.RenameUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -167,11 +168,8 @@ public class TagManager implements TagDispatcher {
"Tag with name %s under metalake %s does not exist", name, metalake);
} catch (EntityAlreadyExistsException e) {
String newName =
- Arrays.stream(changes)
- .filter(c -> c instanceof TagChange.RenameTag)
- .map(c -> ((TagChange.RenameTag) c).getNewName())
- .findFirst()
- .orElse(name);
+ RenameUtils.getLastRenameName(
+ name, changes, TagChange.RenameTag.class,
TagChange.RenameTag::getNewName);
throw new TagAlreadyExistsException(
e, "Tag with name %s under metalake %s already exists", newName,
metalake);
} catch (OptimisticLockException ole) {
diff --git a/core/src/main/java/org/apache/gravitino/utils/RenameUtils.java
b/core/src/main/java/org/apache/gravitino/utils/RenameUtils.java
new file mode 100644
index 0000000000..34df8c7da0
--- /dev/null
+++ b/core/src/main/java/org/apache/gravitino/utils/RenameUtils.java
@@ -0,0 +1,51 @@
+/*
+ * 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.gravitino.utils;
+
+import java.util.function.Function;
+
+/** Utilities for resolving names from ordered rename changes. */
+public final class RenameUtils {
+
+ private RenameUtils() {}
+
+ /**
+ * Returns the name from the last rename change, or the original name if
there is no rename.
+ *
+ * <p>This resolves only the name, not namespace changes such as moving a
table to another schema.
+ *
+ * @param originalName the name before applying changes
+ * @param changes the changes in application order
+ * @param renameType the type of rename change
+ * @param nameExtractor extracts the new name from a rename change
+ * @param <C> the change type
+ * @param <R> the rename change type
+ * @return the final name after applying the rename changes
+ */
+ public static <C, R extends C> String getLastRenameName(
+ String originalName, C[] changes, Class<R> renameType, Function<R,
String> nameExtractor) {
+ String name = originalName;
+ for (C change : changes) {
+ if (renameType.isInstance(change)) {
+ name = nameExtractor.apply(renameType.cast(change));
+ }
+ }
+ return name;
+ }
+}
diff --git
a/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
b/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
index 8153929910..72c9a52d29 100644
--- a/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
+++ b/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
@@ -65,6 +65,7 @@ import org.apache.gravitino.Catalog;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
import org.apache.gravitino.GravitinoEnv;
@@ -349,6 +350,32 @@ public class TestPolicyManager {
e.getMessage());
}
+ @Test
+ void testRenamePolicyToExistingName() {
+ PolicyContent content =
+ PolicyContents.custom(ImmutableMap.of("rule", "value"),
SUPPORTS_OBJECT_TYPES, null);
+ PolicyEntity source = createCustomPolicy(METALAKE, "source", content);
+ PolicyEntity target = createCustomPolicy(METALAKE, "target", content);
+ for (PolicyChange[] changes :
+ new PolicyChange[][] {
+ {PolicyChange.rename("target")},
+ {PolicyChange.rename("intermediate"), PolicyChange.rename("target")}
+ }) {
+ PolicyAlreadyExistsException exception =
+ Assertions.assertThrows(
+ PolicyAlreadyExistsException.class,
+ () -> policyManager.alterPolicy(METALAKE, "source", changes));
+ Assertions.assertEquals(
+ "Policy with name target under metalake " + METALAKE + " already
exists",
+ exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ Assertions.assertEquals(source.id(), policyManager.getPolicy(METALAKE,
"source").id());
+ Assertions.assertEquals(target.id(), policyManager.getPolicy(METALAKE,
"target").id());
+ Assertions.assertThrows(
+ NoSuchPolicyException.class, () -> policyManager.getPolicy(METALAKE,
"intermediate"));
+ }
+ }
+
@Test
public void testCreateAndListPolicies() {
String policyName1 = "policy1" + UUID.randomUUID().toString().replace("-",
"");
diff --git
a/core/src/test/java/org/apache/gravitino/storage/relational/TestRenameConflicts.java
b/core/src/test/java/org/apache/gravitino/storage/relational/TestRenameConflicts.java
new file mode 100644
index 0000000000..81fdc86e43
--- /dev/null
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/TestRenameConflicts.java
@@ -0,0 +1,311 @@
+/*
+ * 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.gravitino.storage.relational;
+
+import static org.apache.gravitino.TestCatalog.PROPERTY_KEY1;
+import static org.apache.gravitino.TestCatalog.PROPERTY_KEY2;
+import static org.apache.gravitino.TestCatalog.PROPERTY_KEY5_PREFIX;
+
+import java.util.Collections;
+import java.util.Map;
+import org.apache.commons.lang3.reflect.FieldUtils;
+import org.apache.gravitino.Catalog;
+import org.apache.gravitino.CatalogChange;
+import org.apache.gravitino.Config;
+import org.apache.gravitino.Configs;
+import org.apache.gravitino.Entity.EntityType;
+import org.apache.gravitino.EntityAlreadyExistsException;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.GravitinoEnv;
+import org.apache.gravitino.MetalakeChange;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.cache.NoOpsCache;
+import org.apache.gravitino.catalog.CatalogManager;
+import org.apache.gravitino.catalog.ManagedTableOperations;
+import org.apache.gravitino.connector.SupportsSchemas;
+import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
+import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException;
+import org.apache.gravitino.exceptions.TableAlreadyExistsException;
+import org.apache.gravitino.lock.LockManager;
+import org.apache.gravitino.meta.BaseMetalake;
+import org.apache.gravitino.meta.CatalogEntity;
+import org.apache.gravitino.meta.TableEntity;
+import org.apache.gravitino.metalake.MetalakeManager;
+import org.apache.gravitino.rel.TableChange;
+import org.apache.gravitino.rel.expressions.distributions.Distributions;
+import org.apache.gravitino.rel.expressions.sorts.SortOrder;
+import org.apache.gravitino.rel.expressions.transforms.Transform;
+import org.apache.gravitino.rel.indexes.Indexes;
+import org.apache.gravitino.secret.SecretManager;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.storage.RandomIdGenerator;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.TestTemplate;
+import org.mockito.Mockito;
+
+/** Verifies rename conflict translation against real metadata database
constraints. */
+public class TestRenameConflicts extends TestJDBCBackend {
+ private Config config;
+ private RelationalEntityStore store;
+ private Object previousConfig;
+ private Object previousLockManager;
+ private LockManager lockManager;
+ private EntityChangeLogPoller poller;
+
+ @BeforeEach
+ void setUpManagers() throws IllegalAccessException {
+ config = new Config(false) {};
+ config.set(Configs.CACHE_ENABLED, false);
+ config.set(Configs.CATALOG_LOAD_ISOLATED, false);
+ previousConfig = FieldUtils.readField(GravitinoEnv.getInstance(),
"config", true);
+ previousLockManager = FieldUtils.readField(GravitinoEnv.getInstance(),
"lockManager", true);
+ if (lockManager == null) {
+ lockManager = new LockManager(config);
+ }
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager",
lockManager, true);
+ store = new RelationalEntityStore();
+ FieldUtils.writeField(store, "backend", backend, true);
+ FieldUtils.writeField(store, "cache", new NoOpsCache(config), true);
+ poller = new EntityChangeLogPoller(3600);
+ FieldUtils.writeField(store, "entityChangeLogPoller", poller, true);
+ }
+
+ @AfterEach
+ void restoreEnvironment() throws Exception {
+ poller.close();
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "config",
previousConfig, true);
+ FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager",
previousLockManager, true);
+ }
+
+ @TestTemplate
+ void testRenameMetalakeToExistingName() throws Exception {
+ BaseMetalake source = createAndInsertMakeLake("source");
+ BaseMetalake target = createAndInsertMakeLake("target");
+ MetalakeManager manager = new MetalakeManager(store,
RandomIdGenerator.INSTANCE);
+ for (MetalakeChange[] changes :
+ new MetalakeChange[][] {
+ {MetalakeChange.rename("target")},
+ {MetalakeChange.rename("intermediate"),
MetalakeChange.rename("target")}
+ }) {
+ MetalakeAlreadyExistsException exception =
+ Assertions.assertThrows(
+ MetalakeAlreadyExistsException.class,
+ () -> manager.alterMetalake(source.nameIdentifier(), changes));
+ Assertions.assertEquals("Metalake target already exists",
exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ Assertions.assertEquals(source.id(),
manager.loadMetalake(source.nameIdentifier()).id());
+ Assertions.assertEquals(target.id(),
manager.loadMetalake(target.nameIdentifier()).id());
+ Assertions.assertFalse(store.exists(NameIdentifier.of("intermediate"),
EntityType.METALAKE));
+ }
+ }
+
+ @TestTemplate
+ void testRenameCatalogToExistingName() throws Exception {
+ createAndInsertMakeLake("lake");
+ NameIdentifier source = NameIdentifier.of("lake", "source");
+ NameIdentifier target = NameIdentifier.of("lake", "target");
+ Map<String, String> properties =
+ Map.of(
+ "provider",
+ "test",
+ PROPERTY_KEY1,
+ "value1",
+ PROPERTY_KEY2,
+ "value2",
+ PROPERTY_KEY5_PREFIX + "1",
+ "value3");
+ try (CatalogManager manager =
+ new CatalogManager(config, store, RandomIdGenerator.INSTANCE, new
SecretManager(config))) {
+ manager.createCatalog(source, Catalog.Type.RELATIONAL, "test", "source
comment", properties);
+ manager.createCatalog(target, Catalog.Type.RELATIONAL, "test", "target
comment", properties);
+ CatalogEntity originalSource = store.get(source, EntityType.CATALOG,
CatalogEntity.class);
+ CatalogEntity originalTarget = store.get(target, EntityType.CATALOG,
CatalogEntity.class);
+ for (CatalogChange[] changes :
+ new CatalogChange[][] {
+ {CatalogChange.rename("target")},
+ {CatalogChange.rename("intermediate"),
CatalogChange.rename("target")}
+ }) {
+ CatalogAlreadyExistsException exception =
+ Assertions.assertThrows(
+ CatalogAlreadyExistsException.class, () ->
manager.alterCatalog(source, changes));
+ Assertions.assertEquals("Catalog " + target + " already exists",
exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ Assertions.assertEquals(
+ originalSource.id(), store.get(source, EntityType.CATALOG,
CatalogEntity.class).id());
+ Assertions.assertEquals(
+ originalTarget.id(), store.get(target, EntityType.CATALOG,
CatalogEntity.class).id());
+ Assertions.assertEquals("source comment",
manager.loadCatalog(source).comment());
+ Assertions.assertEquals("target comment",
manager.loadCatalog(target).comment());
+ Assertions.assertFalse(
+ store.exists(NameIdentifier.of("lake", "intermediate"),
EntityType.CATALOG));
+ }
+ }
+ }
+
+ @TestTemplate
+ void testRenameManagedTableToExistingName() throws Exception {
+ createParentEntities("lake", "catalog", "schema", AUDIT_INFO);
+ createAndInsertSchema("lake", "catalog", "other_schema");
+ TableEntity source = table(Namespace.of("lake", "catalog", "schema"),
"source");
+ TableEntity target = table(source.namespace(), "target");
+ TableEntity crossSchemaTarget =
+ table(Namespace.of("lake", "catalog", "other_schema"), "target");
+ backend.insert(source, false);
+ backend.insert(target, false);
+ backend.insert(crossSchemaTarget, false);
+ ManagedTableOperations operations = managedTableOperations();
+ record RenameCase(TableEntity expectedTarget, TableChange... changes) {}
+ for (RenameCase renameCase :
+ new RenameCase[] {
+ new RenameCase(target, TableChange.rename("target")),
+ new RenameCase(target, TableChange.rename("intermediate"),
TableChange.rename("target")),
+ new RenameCase(crossSchemaTarget, TableChange.rename("target",
"other_schema")),
+ new RenameCase(
+ crossSchemaTarget,
+ TableChange.rename("intermediate", "other_schema"),
+ TableChange.rename("target")),
+ new RenameCase(
+ target,
+ TableChange.rename("intermediate", "other_schema"),
+ TableChange.rename("intermediate", "schema"),
+ TableChange.rename("target"))
+ }) {
+ TableAlreadyExistsException exception =
+ Assertions.assertThrows(
+ TableAlreadyExistsException.class,
+ () -> operations.alterTable(source.nameIdentifier(),
renameCase.changes()));
+ Assertions.assertEquals(
+ "Table " + renameCase.expectedTarget().nameIdentifier() + " already
exists",
+ exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ Assertions.assertEquals(
+ source.id(),
+ store.get(source.nameIdentifier(), EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertEquals(
+ target.id(),
+ store.get(target.nameIdentifier(), EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertEquals(
+ crossSchemaTarget.id(),
+ store.get(crossSchemaTarget.nameIdentifier(), EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertFalse(
+ store.exists(NameIdentifier.of(source.namespace(), "intermediate"),
EntityType.TABLE));
+ Assertions.assertFalse(
+ store.exists(
+ NameIdentifier.of(crossSchemaTarget.namespace(),
"intermediate"), EntityType.TABLE));
+ }
+ }
+
+ @TestTemplate
+ void testRenameManagedTableKeepsEarlierSchema() throws Exception {
+ createParentEntities("lake", "catalog", "schema", AUDIT_INFO);
+ createAndInsertSchema("lake", "catalog", "other_schema");
+ TableEntity source = table(Namespace.of("lake", "catalog", "schema"),
"source");
+ TableEntity originalSchemaTarget = table(source.namespace(), "target");
+ backend.insert(source, false);
+ backend.insert(originalSchemaTarget, false);
+
+ managedTableOperations()
+ .alterTable(
+ source.nameIdentifier(),
+ TableChange.rename("intermediate", "other_schema"),
+ TableChange.rename("target"));
+
+ NameIdentifier expectedTarget = NameIdentifier.of("lake", "catalog",
"other_schema", "target");
+ Assertions.assertEquals(
+ source.id(), store.get(expectedTarget, EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertEquals(
+ originalSchemaTarget.id(),
+ store.get(originalSchemaTarget.nameIdentifier(), EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertFalse(store.exists(source.nameIdentifier(),
EntityType.TABLE));
+ Assertions.assertFalse(
+ store.exists(
+ NameIdentifier.of("lake", "catalog", "other_schema",
"intermediate"),
+ EntityType.TABLE));
+ }
+
+ @TestTemplate
+ void testRenameManagedTableBackToOriginalSchema() throws Exception {
+ createParentEntities("lake", "catalog", "schema", AUDIT_INFO);
+ createAndInsertSchema("lake", "catalog", "other_schema");
+ TableEntity source = table(Namespace.of("lake", "catalog", "schema"),
"source");
+ TableEntity crossSchemaTarget =
+ table(Namespace.of("lake", "catalog", "other_schema"), "target");
+ backend.insert(source, false);
+ backend.insert(crossSchemaTarget, false);
+
+ managedTableOperations()
+ .alterTable(
+ source.nameIdentifier(),
+ TableChange.rename("intermediate", "other_schema"),
+ TableChange.rename("intermediate", "schema"),
+ TableChange.rename("target"));
+
+ NameIdentifier expectedTarget = NameIdentifier.of(source.namespace(),
"target");
+ Assertions.assertEquals(
+ source.id(), store.get(expectedTarget, EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertEquals(
+ crossSchemaTarget.id(),
+ store.get(crossSchemaTarget.nameIdentifier(), EntityType.TABLE,
TableEntity.class).id());
+ Assertions.assertFalse(store.exists(source.nameIdentifier(),
EntityType.TABLE));
+ Assertions.assertFalse(
+ store.exists(NameIdentifier.of(source.namespace(), "intermediate"),
EntityType.TABLE));
+ Assertions.assertFalse(
+ store.exists(
+ NameIdentifier.of("lake", "catalog", "other_schema",
"intermediate"),
+ EntityType.TABLE));
+ }
+
+ private ManagedTableOperations managedTableOperations() {
+ return new ManagedTableOperations() {
+ @Override
+ protected EntityStore store() {
+ return store;
+ }
+
+ @Override
+ protected SupportsSchemas schemas() {
+ return Mockito.mock(SupportsSchemas.class);
+ }
+
+ @Override
+ protected IdGenerator idGenerator() {
+ return RandomIdGenerator.INSTANCE;
+ }
+ };
+ }
+
+ private TableEntity table(Namespace namespace, String name) {
+ return TableEntity.builder()
+ .withId(RandomIdGenerator.INSTANCE.nextId())
+ .withName(name)
+ .withNamespace(namespace)
+ .withProperties(Collections.emptyMap())
+ .withColumns(Collections.emptyList())
+ .withPartitioning(new Transform[0])
+ .withSortOrders(new SortOrder[0])
+ .withIndexes(Indexes.EMPTY_INDEXES)
+ .withDistribution(Distributions.NONE)
+ .withAuditInfo(AUDIT_INFO)
+ .build();
+ }
+}
diff --git a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
index cd5921cdf6..a57eb35e2e 100644
--- a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
+++ b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
@@ -65,6 +65,7 @@ import org.apache.gravitino.Catalog;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
@@ -538,6 +539,37 @@ public class TestTagManager {
Assertions.assertEquals("tag2", tagManager.getTag(METALAKE,
"tag2").name());
}
+ @Test
+ void testAlterTagMultipleRenamesToExistingTag() {
+ Tag source = tagManager.createTag(METALAKE, "tag1", "source",
ImmutableMap.of("key", "source"));
+ Tag target = tagManager.createTag(METALAKE, "tag2", "target",
ImmutableMap.of("key", "target"));
+
+ TagAlreadyExistsException exception =
+ Assertions.assertThrows(
+ TagAlreadyExistsException.class,
+ () ->
+ tagManager.alterTag(
+ METALAKE,
+ "tag1",
+ TagChange.rename("intermediate"),
+ TagChange.updateComment("changed"),
+ TagChange.setProperty("key", "changed"),
+ TagChange.rename("tag2")));
+
+ Assertions.assertEquals(
+ "Tag with name tag2 under metalake metalake_for_tag_test already
exists",
+ exception.getMessage());
+ Assertions.assertInstanceOf(EntityAlreadyExistsException.class,
exception.getCause());
+ for (Tag original : new Tag[] {source, target}) {
+ Tag unchanged = tagManager.getTag(METALAKE, original.name());
+ Assertions.assertEquals(original.name(), unchanged.name());
+ Assertions.assertEquals(original.comment(), unchanged.comment());
+ Assertions.assertEquals(original.properties(), unchanged.properties());
+ }
+ Assertions.assertThrows(
+ NoSuchTagException.class, () -> tagManager.getTag(METALAKE,
"intermediate"));
+ }
+
@Test
public void testDeleteTag() {
tagManager.createTag(METALAKE, "tag1", null, null);
diff --git a/core/src/test/java/org/apache/gravitino/utils/TestRenameUtils.java
b/core/src/test/java/org/apache/gravitino/utils/TestRenameUtils.java
new file mode 100644
index 0000000000..8abc37c113
--- /dev/null
+++ b/core/src/test/java/org/apache/gravitino/utils/TestRenameUtils.java
@@ -0,0 +1,63 @@
+/*
+ * 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.gravitino.utils;
+
+import org.apache.gravitino.tag.TagChange;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+class TestRenameUtils {
+
+ @Test
+ void testEmptyChangesKeepOriginalName() {
+ Assertions.assertEquals(
+ "original",
+ RenameUtils.getLastRenameName(
+ "original",
+ new TagChange[0],
+ TagChange.RenameTag.class,
+ TagChange.RenameTag::getNewName));
+ }
+
+ @Test
+ void testNonRenameChangesKeepOriginalName() {
+ TagChange[] changes = {
+ TagChange.updateComment("comment"), TagChange.setProperty("key", "value")
+ };
+ Assertions.assertEquals(
+ "original",
+ RenameUtils.getLastRenameName(
+ "original", changes, TagChange.RenameTag.class,
TagChange.RenameTag::getNewName));
+ }
+
+ @Test
+ void testLastRenameWinsWithOtherChanges() {
+ TagChange[] changes = {
+ TagChange.updateComment("comment"),
+ TagChange.rename("first"),
+ TagChange.setProperty("key", "value"),
+ TagChange.rename("last"),
+ TagChange.removeProperty("key")
+ };
+ Assertions.assertEquals(
+ "last",
+ RenameUtils.getLastRenameName(
+ "original", changes, TagChange.RenameTag.class,
TagChange.RenameTag::getNewName));
+ }
+}
diff --git a/docs/open-api/models.yaml b/docs/open-api/models.yaml
index 08446ca984..7a82d411a1 100644
--- a/docs/open-api/models.yaml
+++ b/docs/open-api/models.yaml
@@ -133,6 +133,15 @@ paths:
examples:
NoSuchModelException:
$ref: "#/components/examples/NoSuchModelException"
+ "409":
+ description: Conflict - The target model already exists
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ ModelAlreadyExistsErrorResponse:
+ $ref: "#/components/examples/ModelAlreadyExistsException"
"5xx":
$ref: "./openapi.yaml#/components/responses/ServerErrorResponse"
diff --git a/docs/open-api/policies.yaml b/docs/open-api/policies.yaml
index ad0cbdc5b4..8f8a9f1a61 100644
--- a/docs/open-api/policies.yaml
+++ b/docs/open-api/policies.yaml
@@ -160,6 +160,15 @@ paths:
$ref:
"./metalakes.yaml#/components/examples/NoSuchMetalakeException"
NoSuchPolicyException:
$ref: "#/components/examples/NoSuchPolicyException"
+ "409":
+ description: Conflict - The target policy already exists
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ PolicyAlreadyExistsErrorResponse:
+ $ref: "#/components/examples/PolicyAlreadyExistsException"
"5xx":
$ref: "./openapi.yaml#/components/responses/ServerErrorResponse"
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
index 16ac02e0d5..d4a90a3642 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
@@ -571,6 +571,27 @@ public class TestCatalogOperations extends
BaseOperationsTest {
assertNullRequestBodyRejected(resp);
}
+ @Test
+ void testRenameCatalogToExistingName() {
+ CatalogAlreadyExistsException exception =
+ new CatalogAlreadyExistsException("Catalog existing already exists");
+ doThrow(exception).when(manager).alterCatalog(any(), any());
+ CatalogUpdatesRequest request =
+ new CatalogUpdatesRequest(
+ Collections.singletonList(new
CatalogUpdateRequest.RenameCatalogRequest("existing")));
+ try (Response response =
+ target("/metalakes/metalake1/catalogs/catalog1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
error.getCode());
+
Assertions.assertEquals(CatalogAlreadyExistsException.class.getSimpleName(),
error.getType());
+
Assertions.assertTrue(error.getMessage().contains(exception.getMessage()));
+ }
+ }
+
@Test
public void testAlterCatalog() {
TestCatalog catalog = buildCatalog("metalake1", "catalog2");
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
index 6a7b10af98..9b99a7efae 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java
@@ -34,6 +34,7 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import java.io.IOException;
import java.time.Instant;
+import java.util.Collections;
import java.util.Map;
import java.util.stream.Stream;
import javax.servlet.http.HttpServletRequest;
@@ -452,6 +453,27 @@ public class TestFilesetOperations extends
BaseOperationsTest {
assertNullRequestBodyRejected(resp);
}
+ @Test
+ void testRenameFilesetToExistingName() {
+ FilesetAlreadyExistsException exception =
+ new FilesetAlreadyExistsException("Fileset existing already exists");
+ doThrow(exception).when(dispatcher).alterFileset(any(),
any(FilesetChange.class));
+ FilesetUpdatesRequest request =
+ new FilesetUpdatesRequest(
+ Collections.singletonList(new
FilesetUpdateRequest.RenameFilesetRequest("existing")));
+ try (Response response =
+ target(filesetPath(metalake, catalog, schema) + "fileset1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
error.getCode());
+
Assertions.assertEquals(FilesetAlreadyExistsException.class.getSimpleName(),
error.getType());
+
Assertions.assertTrue(error.getMessage().contains(exception.getMessage()));
+ }
+ }
+
@Test
public void testRenameFileset() {
FilesetUpdateRequest req = new
FilesetUpdateRequest.RenameFilesetRequest("new name");
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
index a2cbd0b251..6eeb2b7669 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestMetalakeOperations.java
@@ -37,6 +37,7 @@ import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import java.io.IOException;
import java.time.Instant;
+import java.util.Collections;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.GET;
@@ -59,6 +60,7 @@ import org.apache.gravitino.dto.responses.ErrorConstants;
import org.apache.gravitino.dto.responses.ErrorResponse;
import org.apache.gravitino.dto.responses.MetalakeListResponse;
import org.apache.gravitino.dto.responses.MetalakeResponse;
+import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.lock.LockManager;
import org.apache.gravitino.meta.AuditInfo;
@@ -401,6 +403,28 @@ public class TestMetalakeOperations extends
BaseOperationsTest {
.contains("Failed to operate object [" + metalakeName + "]
operation [LOAD]"));
}
+ @Test
+ void testRenameMetalakeToExistingName() {
+ MetalakeAlreadyExistsException exception =
+ new MetalakeAlreadyExistsException("Metalake existing already exists");
+ doThrow(exception).when(metalakeManager).alterMetalake(any(),
any(MetalakeChange.class));
+ MetalakeUpdatesRequest request =
+ new MetalakeUpdatesRequest(
+ Collections.singletonList(new
MetalakeUpdateRequest.RenameMetalakeRequest("existing")));
+ try (Response response =
+ target("/metalakes/test")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
error.getCode());
+ Assertions.assertEquals(
+ MetalakeAlreadyExistsException.class.getSimpleName(),
error.getType());
+
Assertions.assertTrue(error.getMessage().contains(exception.getMessage()));
+ }
+ }
+
@Test
public void testAlterMetalake() {
String metalakeName = "test";
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java
index 0eb656f118..5d09a1f175 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java
@@ -20,6 +20,7 @@ package org.apache.gravitino.server.web.rest;
import static org.apache.gravitino.Configs.CACHE_ENABLED;
import static org.apache.gravitino.Configs.ENABLE_AUTHORIZATION;
+import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
@@ -1154,6 +1155,27 @@ public class TestModelOperations extends
BaseOperationsTest {
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResp2.getType());
}
+ @Test
+ void testRenameModelToExistingName() {
+ ModelAlreadyExistsException exception =
+ new ModelAlreadyExistsException("Model existing already exists");
+ doThrow(exception).when(modelDispatcher).alterModel(any(),
any(ModelChange.class));
+ ModelUpdatesRequest request =
+ new ModelUpdatesRequest(
+ Collections.singletonList(new
ModelUpdateRequest.RenameModelRequest("existing")));
+ try (Response response =
+ target(modelPath() + "/model1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
error.getCode());
+
Assertions.assertEquals(ModelAlreadyExistsException.class.getSimpleName(),
error.getType());
+
Assertions.assertTrue(error.getMessage().contains(exception.getMessage()));
+ }
+ }
+
@Test
public void testRenameModel() {
String oldName = "model1";
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
index 1d53af9576..0063630f7b 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java
@@ -33,6 +33,7 @@ import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import java.io.IOException;
import java.time.Instant;
+import java.util.Collections;
import java.util.Optional;
import javax.servlet.http.HttpServletRequest;
import javax.ws.rs.client.Entity;
@@ -585,6 +586,27 @@ public class TestPolicyOperations extends
BaseOperationsTest {
assertNullRequestBodyRejected(resp);
}
+ @Test
+ void testRenamePolicyToExistingName() {
+ PolicyAlreadyExistsException exception =
+ new PolicyAlreadyExistsException("Policy existing already exists");
+ doThrow(exception).when(policyManager).alterPolicy(any(), any(),
any(PolicyChange.class));
+ PolicyUpdatesRequest request =
+ new PolicyUpdatesRequest(
+ Collections.singletonList(new
PolicyUpdateRequest.RenamePolicyRequest("existing")));
+ try (Response response =
+ target(policyPath(metalake) + "/policy1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
error.getCode());
+
Assertions.assertEquals(PolicyAlreadyExistsException.class.getSimpleName(),
error.getType());
+
Assertions.assertTrue(error.getMessage().contains(exception.getMessage()));
+ }
+ }
+
@Test
public void testAlterPolicy() {
ImmutableMap<String, Object> contentFields =
ImmutableMap.of("target_file_size_bytes", 1000);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
index 2c8d0fdff1..9864b208d5 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java
@@ -34,6 +34,7 @@ import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.time.Instant;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
@@ -615,6 +616,27 @@ public class TestTableOperations extends
BaseOperationsTest {
assertNullRequestBodyRejected(resp);
}
+ @Test
+ void testRenameTableToExistingName() {
+ TableAlreadyExistsException exception =
+ new TableAlreadyExistsException("Table existing already exists");
+ doThrow(exception).when(dispatcher).alterTable(any(),
any(TableChange.class));
+ TableUpdatesRequest request =
+ new TableUpdatesRequest(
+ Collections.singletonList(new
TableUpdateRequest.RenameTableRequest("existing")));
+ try (Response response =
+ target(tablePath(metalake, catalog, schema) + "table1")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE))) {
+ Assertions.assertEquals(Response.Status.CONFLICT.getStatusCode(),
response.getStatus());
+ ErrorResponse error = response.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ALREADY_EXISTS_CODE,
error.getCode());
+
Assertions.assertEquals(TableAlreadyExistsException.class.getSimpleName(),
error.getType());
+
Assertions.assertTrue(error.getMessage().contains(exception.getMessage()));
+ }
+ }
+
@Test
public void testRenameTable() {
TableUpdateRequest.RenameTableRequest req = new
TableUpdateRequest.RenameTableRequest("table2");
