mchades commented on code in PR #14040:
URL: https://github.com/apache/gravitino/pull/14040#discussion_r4335365794
##########
core/src/main/java/org/apache/gravitino/policy/PolicyManager.java:
##########
@@ -161,10 +162,14 @@ public PolicyEntity alterPolicy(String metalake, String
policyName, PolicyChange
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 =
+ Arrays.stream(changes)
+ .filter(c -> c instanceof PolicyChange.RenamePolicy)
+ .map(c -> ((PolicyChange.RenamePolicy) c).getNewName())
+ .reduce((first, second) -> second)
+ .orElse(policyName);
+ throw new PolicyAlreadyExistsException(
+ e, "Policy with name %s under metalake %s already exists", newName,
metalake);
Review Comment:
This duplicates your [earlier OpenAPI 409
comment](https://github.com/apache/gravitino/pull/14040#discussion_r4333721143).
Both responses are now documented, and `./gradlew :docs:build` passes.
##########
core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java:
##########
@@ -181,8 +181,26 @@ public Table alterTable(NameIdentifier ident,
TableChange... changes)
} 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);
+ NameIdentifier newIdent =
+ Arrays.stream(changes)
+ .filter(c -> c instanceof TableChange.RenameTable)
+ .map(c -> (TableChange.RenameTable) c)
+ .map(
+ rename ->
+ NameIdentifier.of(
+ rename
+ .getNewSchemaName()
+ .map(
+ schema ->
+ NamespaceUtil.ofTable(
+ ident.namespace().level(0),
+ ident.namespace().level(1),
+ schema))
+ .orElse(ident.namespace()),
+ rename.getNewName()))
+ .reduce((first, second) -> second)
Review Comment:
This duplicates your [earlier comment on target derivation and schema
handling](https://github.com/apache/gravitino/pull/14040#discussion_r4333721417).
The fix preserves the accumulated schema and reuses the updater's final
identifier.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
