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]