yuqi1129 commented on code in PR #14336:
URL: https://github.com/apache/gravitino/pull/14336#discussion_r4409396269
##########
core/src/main/java/org/apache/gravitino/catalog/TableOperationDispatcher.java:
##########
@@ -977,12 +977,23 @@ private Pair<Boolean, List<ColumnEntity>>
updateColumnsIfNecessary(
return Pair.of(false, Collections.emptyList());
}
+ // A catalog that reports no column list says nothing about the columns,
so keep the stored
+ // ones. Treating it as "all columns dropped" would delete their tags,
policies and owners.
+ if (tableFromCatalog.columns() == null) {
Review Comment:
Follow-up in a56c9b4bbb: the check also runs on the unstored alter path,
where Gravitino stores nothing, and on the managed path, where the catalog
itself has already updated the store.
`testAlterUnstoredTableFailsWhenCatalogReportsNoColumns` covers the unstored
one; the managed one has no test. Since the alter has already taken effect when
it fails, the hook's authorization rename and the success event are skipped, an
`AlterTableFailureEvent` is emitted, and a retried rename fails because the
table already has its new name. These are now documented in the code comment
and the PR description.
--
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]
