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.  is restored. The Iceberg capability is now 
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.  also resolves an unambiguous Hive table alias to the 
stored physical spelling before dispatch. The regression now additionally 
executes uppercase  to cover the  path. The targeted Hive-backed Iceberg test 
passed on both Spark 3.5 and Spark 4.0.



##########
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  still refers to a 
soft-deleted record (). 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.  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.  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  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]

Reply via email to