Copilot commented on code in PR #11555:
URL: https://github.com/apache/gravitino/pull/11555#discussion_r3386238505
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueIcebergTableHelper.java:
##########
@@ -391,9 +392,22 @@ static void createTable(
* @param dbName the Glue database name
* @param tableName the table name
* @param changes the table changes to apply
+ * @return the final table name (differs from {@code tableName} only when a
rename is applied)
*/
- static void alterTable(
+ static String alterTable(
Catalog icebergCatalog, String dbName, String tableName, TableChange...
changes) {
+ for (TableChange change : changes) {
+ if (change instanceof TableChange.RenameTable) {
+ Preconditions.checkArgument(
+ changes.length == 1, "RenameTable cannot be combined with other
table changes");
+ String newName = ((TableChange.RenameTable) change).getNewName();
+ icebergCatalog.renameTable(
+ TableIdentifier.of(dbName, tableName), TableIdentifier.of(dbName,
newName));
+ LOG.info("Renamed Iceberg table {}.{} to {}", dbName, tableName,
newName);
+ return newName;
+ }
Review Comment:
`TableChange.RenameTable` can include an optional `newSchemaName` (see
`TableChange.rename(newName, newSchemaName)`), but this implementation ignores
it and always renames within the current `dbName`. If a client sends a rename
that moves the table to a different schema, this will do the wrong thing (or
rename to an unexpected target). Either honor `newSchemaName` in the Iceberg
`TableIdentifier`, or explicitly reject cross-schema renames here.
--
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]
