roryqi commented on code in PR #13771:
URL: https://github.com/apache/gravitino/pull/13771#discussion_r4256054759
##########
spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/integration/test/SparkCommonIT.java:
##########
@@ -630,7 +630,7 @@ protected void testAlterTableReplaceColumns() {
tableInfo = getTableInfo(tableName);
checkTableColumns(tableName, updateColumns, tableInfo);
- sql(String.format("INSERT INTO %S VALUES(3, 'name2', 10)", tableName));
+ sql(String.format("INSERT INTO %s VALUES(3, 'name2', 10)", tableName));
Review Comment:
Addressed in 25297d4308 and simplified further in ca27d8c608. `%S` is
restored. The Iceberg capability is backend-aware: Hive-backed catalogs fold
SCHEMA and TABLE names, while COLUMN remains case-sensitive because column
semantics belong to Iceberg rather than the catalog backend. No
`resolveTableName` implementation is needed; Capability normalization alone
handles the alias. The regression also executes uppercase `CREATE TABLE IF NOT
EXISTS` to cover the `tableExists` path. The targeted Hive-backed Iceberg test
passed on Spark 3.5 and Spark 4.0, and passed again on Spark 3.5 after removing
the resolver.
##########
core/src/main/java/org/apache/gravitino/catalog/TopicOperationDispatcher.java:
##########
@@ -309,6 +309,10 @@ private void importTopic(NameIdentifier identifier) {
.build();
try {
+ // Kafka does not persist a removable Gravitino identifier. Its
immutable topic UUID is
+ // converted to the entity ID on every load, so catalog re-registration
must rebind that ID
+ // to the current Gravitino path. Overwrite is safe for this
Kafka-specific identity model:
+ // Kafka does not support topic rename, and deleting a topic gives a
replacement a new UUID.
Review Comment:
Addressed in 25297d4308 with the narrower recovery path suggested here.
Topic import first performs a create-only write. On an ID conflict, the
relational store atomically updates the row only when `topic_id` still refers
to a soft-deleted record (`WHERE topic_id = ... AND deleted_at > 0`). An active
UUID under another Gravitino path is never rebound; the dispatcher reloads to
distinguish an HA winner at the same path from the multi-path conflict and
reports the immutable Kafka UUID conflict explicitly. `EntityStore.put`
semantics are unchanged. Dispatcher, relational-service, and generated-SQL
tests cover HA reuse, soft-deleted restore, and active-row refusal.
##########
catalogs/catalog-hive/src/test/java/org/apache/gravitino/catalog/hive/integration/test/CatalogHive2IT.java:
##########
@@ -1458,20 +1460,81 @@ public void testOutOfBandRenameKeepsColumnTags() throws
InterruptedException {
return null;
});
- // The column keeps its id, so the tag follows the table to its new name.
+ // Loading the external name now requires manual identifier cleanup. The
rejected import must
+ // leave the tag relation attached to the original Gravitino identity.
NameIdentifier newIdent = NameIdentifier.of(schema, newName);
- Column column = loadColumn(newIdent, HIVE_COL_NAME1);
- Assertions.assertArrayEquals(new String[] {tagName},
column.supportsTags().listTags());
+ UnsupportedOperationException exception =
+ assertThrows(
+ UnsupportedOperationException.class, () -> loadColumn(newIdent,
HIVE_COL_NAME1));
+
Assertions.assertTrue(exception.getMessage().contains("`gravitino.identifier`"));
MetadataObject[] objects =
metalake.getTag(tagName).associatedObjects().objects();
Assertions.assertEquals(1, objects.length);
Assertions.assertEquals(
- String.join(".", catalogName, schema, newName,
HIVE_COL_NAME1).toLowerCase(Locale.ROOT),
+ String.join(".", catalogName, schema, ident.name(), HIVE_COL_NAME1)
+ .toLowerCase(Locale.ROOT),
objects[0].fullName().toLowerCase(Locale.ROOT));
} finally {
metalake.deleteTag(tagName);
}
}
+ @Test
+ public void testCatalogReregistrationRequiresExternalIdentifierCleanup()
Review Comment:
Added in 25297d4308.
`CatalogHive2IT.testCatalogReregistrationRequiresExternalIdentifierCleanup` now
creates a table before dropping the catalog. After re-registration it clears
the schema identifier and imports the schema, verifies that loading the table
still reports its own retained soft-deleted identifier, clears the table's
`gravitino.identifier` in HMS, and then verifies that the table imports
successfully. The targeted Docker-backed Hive integration test passed locally.
--
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]
