lasdf1234 commented on code in PR #14584:
URL: https://github.com/apache/gravitino/pull/14584#discussion_r4477357782
##########
catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogOperations.java:
##########
@@ -511,6 +514,9 @@ private Table renameTable(NameIdentifier tableIdent,
TableChange.RenameTable ren
backend.wrapper().renameTable(renameTableRequest);
return loadTable(
NameIdentifier.of(ArrayUtils.add(destNamespace.levels(),
renameTable.getNewName())));
+ } catch (AlreadyExistsException e) {
+ throw new TableAlreadyExistsException(
+ e, "Table already exists: %s", renameTable.getNewName());
Review Comment:
This rename conflict translation is already on `main` via #14556 (with a
fully-qualified destination name). After rebase this will conflict / be
redundant — please drop this catch and keep only the new `ValidationException`
/ auto-increment changes from this PR. Also remove `Fixes #14419` from the PR
description since that issue is already closed.
##########
iceberg/iceberg-common/src/main/java/org/apache/gravitino/iceberg/common/ops/IcebergCatalogWrapper.java:
##########
@@ -666,4 +672,18 @@ private void validateDefaultSpecAndSortOrder(
}
}
}
+
+ private void validateNamespaceProperties(Map<String, String> properties) {
+ if (getCatalog() instanceof JdbcCatalog) {
+ properties.forEach(
Review Comment:
If `properties` / `updates()` is null, `forEach` will NPE before the length
check. Please null-guard first, e.g. `if (properties == null) { return; }`
(callers pass `request.properties()` and
`updateNamespacePropertiesRequest.updates()`).
The `1000` limit itself looks correct — it matches Iceberg `JdbcUtil` /
`VARCHAR(1000)`.
--
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]
