yuqi1129 commented on code in PR #14336:
URL: https://github.com/apache/gravitino/pull/14336#discussion_r4409395969
##########
common/src/main/java/org/apache/gravitino/dto/util/DTOConverters.java:
##########
@@ -299,7 +299,11 @@ public static TableDTO toDTO(Table table) {
.withName(table.name())
.withComment(table.comment())
.withColumns(
-
Arrays.stream(table.columns()).map(DTOConverters::toDTO).toArray(ColumnDTO[]::new))
+ table.columns() == null
Review Comment:
Follow-up in a56c9b4bbb: `createTable` had the same gap. With the `toDTO`
guard, a create whose catalog returns no column list stored the table and set
its owner before `TableInfo` threw the NPE. It now fails with the same
`IllegalStateException` before storing the table, where the snapshot used to
fail. `testCreateFailsWhenCatalogReportsNoColumns` checks that nothing is
stored.
##########
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 and managed
alter paths, where the store is not touched;
`testAlterUnstoredTableFailsWhenCatalogReportsNoColumns` covers the unstored
one. 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]
