This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 29749776d9 [Cherry-pick to branch-1.3] [#14039] fix(core): Return
consistent conflicts for metadata renames (#14040) (#14137)
29749776d9 is described below
commit 29749776d95d2ecc6c804b27db640982ae2d8e89
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Oct 23 22:54:09 2026 +0800
[Cherry-pick to branch-1.3] [#14039] fix(core): Return consistent conflicts
for metadata renames (#14040) (#14137)
**Cherry-pick Information:**
- Original PR: #14040
- Original commit: `5c8c1d99fed331360e87f5f0730251655398c135`
- Target branch: `branch-1.3`
- Status: Conflicts resolved in
f6656bc1e19dbf4cadeecd92b9c2de75109be337.
Preserves branch-1.3's Policy/Tag tree locks and adapts the backported
tests to its constructors and helpers.
**Validation:** 461 targeted tests passed (Core 87, Fileset 227, Model
36, REST 111), plus Spotless, API Javadoc, and OpenAPI validation.
Docker integration tests were not run.
---------
Co-authored-by: mchades <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
---
.../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 | 18 +-
.../fileset/TestFilesetCatalogOperations.java | 41 +++
.../catalog/model/ModelCatalogOperations.java | 15 +-
.../catalog/model/TestModelCatalogOperations.java | 34 +++
.../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 | 309 +++++++++++++++++++++
.../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, 792 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 1009a1f332..a48c83f24a 100644
--- a/api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
+++ b/api/src/main/java/org/apache/gravitino/SupportsCatalogs.java
@@ -125,10 +125,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 c0c1885917..4bf8e11bd9 100644
--- a/api/src/main/java/org/apache/gravitino/file/FilesetCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/file/FilesetCatalog.java
@@ -140,10 +140,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 e10df19ce2..233d41c83c 100644
--- a/api/src/main/java/org/apache/gravitino/model/ModelCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/model/ModelCatalog.java
@@ -312,10 +312,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 1bbd8062ac..644459654c 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 888526ffd5..a5ed9c88e7 100644
--- a/api/src/main/java/org/apache/gravitino/rel/TableCatalog.java
+++ b/api/src/main/java/org/apache/gravitino/rel/TableCatalog.java
@@ -281,10 +281,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 433c52401b..62db7197de 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
@@ -68,6 +68,7 @@ import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.GravitinoEnv;
@@ -97,7 +98,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;
@@ -123,6 +123,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;
@@ -656,7 +657,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);
@@ -685,10 +686,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));
}
}
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 378dd580b7..da358bcc46 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
@@ -74,6 +74,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.reflect.FieldUtils;
import org.apache.gravitino.Config;
import org.apache.gravitino.Configs;
+import org.apache.gravitino.EntityAlreadyExistsException;
import org.apache.gravitino.EntityFieldLimits;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.EntityStoreFactory;
@@ -99,6 +100,7 @@ import
org.apache.gravitino.credential.CatalogCredentialManager;
import org.apache.gravitino.credential.Credential;
import org.apache.gravitino.credential.CredentialConstants;
import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.apache.gravitino.exceptions.NoSuchFilesetException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
@@ -1155,6 +1157,45 @@ public class TestFilesetCatalogOperations {
}
}
+ @ParameterizedTest
+ @EnumSource(Fileset.Type.class)
+ void testRenameFilesetToExistingName(Fileset.Type type) throws IOException {
+ long testId = generateTestId();
+ String schemaName = "rename_conflict_" + testId;
+ String schemaPath = TEST_ROOT_PATH + "/" + schemaName;
+ createSchema(testId, 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)) {
+ 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 34fcc5c900..6c5aeb0a0d 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
@@ -54,6 +54,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;
@@ -942,6 +943,39 @@ 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(StringIdentifier.fromId(idGenerator.nextId()),
null));
+ ops.registerModel(
+ target,
+ "target comment",
+
StringIdentifier.newPropertiesWithId(StringIdentifier.fromId(idGenerator.nextId()),
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 153e8b7002..b840c8d25f 100644
--- a/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
+++ b/core/src/main/java/org/apache/gravitino/catalog/CatalogManager.java
@@ -118,6 +118,7 @@ import org.apache.gravitino.utils.ExceptionMessages;
import org.apache.gravitino.utils.IsolatedClassLoader;
import org.apache.gravitino.utils.NamespaceUtil;
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;
@@ -1153,11 +1154,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,
@@ -1254,6 +1256,16 @@ public class CatalogManager implements
CatalogDispatcher, Closeable {
LOG.warn("Failed to alter catalog {} with unknown change", ident,
iae);
throw iae;
+ } 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 0e226abcbb..6b48716b0f 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 b2a01b6a06..fca345ad85 100644
--- a/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java
+++ b/core/src/main/java/org/apache/gravitino/metalake/MetalakeManager.java
@@ -56,6 +56,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 22256fba5f..0e9f14349d 100644
--- a/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
+++ b/core/src/main/java/org/apache/gravitino/policy/PolicyManager.java
@@ -50,6 +50,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;
@@ -175,7 +176,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);
return TreeLockUtils.doWithTreeLock(
@@ -192,10 +194,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 49b8e112b4..971001ae6b 100644
--- a/core/src/main/java/org/apache/gravitino/tag/TagManager.java
+++ b/core/src/main/java/org/apache/gravitino/tag/TagManager.java
@@ -53,6 +53,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;
@@ -176,11 +177,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 (IOException ioe) {
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 181af71e4a..fced5fc92b 100644
--- a/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
+++ b/core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java
@@ -57,6 +57,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;
@@ -321,6 +322,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..ce049e2880
--- /dev/null
+++
b/core/src/test/java/org/apache/gravitino/storage/relational/TestRenameConflicts.java
@@ -0,0 +1,309 @@
+/*
+ * 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.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)) {
+ 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 57182308b2..37a2f19ea1 100644
--- a/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
+++ b/core/src/test/java/org/apache/gravitino/tag/TestTagManager.java
@@ -57,6 +57,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;
@@ -498,6 +499,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 8b6325ff49..7513b3bb72 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 3d2c697502..cbd66e52dd 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
@@ -570,6 +570,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 544c02a26f..89c8699c52 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 e69ec2a0b2..3be28dc41d 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;
@@ -1082,6 +1083,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 85e45239fe..72d25bf287 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;
@@ -527,6 +528,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");
