roryqi commented on code in PR #13771:
URL: https://github.com/apache/gravitino/pull/13771#discussion_r4258146866
##########
catalogs/catalog-kafka/src/test/java/org/apache/gravitino/catalog/kafka/integration/test/CatalogKafkaIT.java:
##########
@@ -192,6 +192,35 @@ public void testCatalog() throws ExecutionException,
InterruptedException {
Assertions.assertFalse(adminClient.listTopics().names().get().isEmpty());
}
+ @Test
+ public void testCatalogReregistrationLoadsExistingTopic()
+ throws ExecutionException, InterruptedException {
+ String catalogName =
GravitinoITUtils.genRandomName("test_catalog_reregistration");
+ String topicName =
GravitinoITUtils.genRandomName("test_topic_reregistration");
+ NameIdentifier topicIdent = NameIdentifier.of(DEFAULT_SCHEMA_NAME,
topicName);
+ Map<String, String> properties = ImmutableMap.of(BOOTSTRAP_SERVERS,
kafkaBootstrapServers);
+
+ adminClient.createTopics(ImmutableList.of(new NewTopic(topicName, 1,
(short) 1))).all().get();
+
+ Catalog originalCatalog = createCatalog(catalogName, "original catalog",
properties);
+ Assertions.assertEquals(
+ topicName,
originalCatalog.asTopicCatalog().loadTopic(topicIdent).name());
+
+ Assertions.assertTrue(metalake.dropCatalog(catalogName, true));
+ Assertions.assertTrue(topicExistsInKafka(topicName));
+
+ Catalog reregisteredCatalog = createCatalog(catalogName, "re-registered
catalog", properties);
+ Topic restoredTopic =
+ Assertions.assertDoesNotThrow(
+ () -> reregisteredCatalog.asTopicCatalog().loadTopic(topicIdent));
+ Assertions.assertEquals(topicName, restoredTopic.name());
+ checkTopicReadWrite(topicName);
Review Comment:
Added the real active-registration integration case in 594b2d2e6e. The test
creates two Kafka catalogs against the same broker while the first registration
remains active, verifies that loading the same topic through the second catalog
is refused with the immutable-UUID error, and then reloads it through the
original catalog to confirm that the active entity was not rebound. The focused
Docker IT passed locally.
##########
catalogs/catalog-lakehouse-iceberg/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergCatalogCapability.java:
##########
@@ -48,6 +60,15 @@ public CapabilityResult supportsHierarchicalSchema() {
return CapabilityResult.SUPPORTED;
}
+ @Override
+ public CapabilityResult caseSensitiveOnName(Scope scope) {
+ if (hiveBackend && (scope == Scope.SCHEMA || scope == Scope.TABLE || scope
== Scope.VIEW)) {
+ return CapabilityResult.unsupported(
+ "The Iceberg Hive backend is case insensitive for schemas, tables,
and views.");
Review Comment:
Fixed in 594b2d2e6e. Native now folds , and verifies that a view created
with a lowercase name can be loaded through its uppercase spelling against HMS.
I also applied the same backend-scoped behavior to Paimon: only folds
schema/table/view names, while columns and non-Hive backends retain their
existing case-sensitive behavior.
##########
spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/integration/test/SparkCommonIT.java:
##########
@@ -630,7 +630,8 @@ protected void testAlterTableReplaceColumns() {
tableInfo = getTableInfo(tableName);
checkTableColumns(tableName, updateColumns, tableInfo);
- sql(String.format("INSERT INTO %S VALUES(3, 'name2', 10)", tableName));
+ sql(String.format("CREATE TABLE IF NOT EXISTS %s (id int, name2 string,
age long)", tableName));
+ sql(String.format("INSERT INTO %s VALUES(3, 'name2', 10)", tableName));
Review Comment:
Fixed in 594b2d2e6e. The shared REPLACE COLUMNS test again performs both
and through an uppercase table name, so native Hive, direct Iceberg-on-Hive,
and REST-routing retain mixed-case coverage. Only overrides the access to use
the original case, because a Gravitino catalog configured with cannot infer
the downstream REST catalog's case-sensitivity; the REPLACE COLUMNS assertions
still run. REST-routing remains covered because its Gravitino catalog is
explicitly configured with . I also removed the now-redundant dedicated
direct-Hive test. All four focused Spark 3.5 tests passed locally.
##########
spark-connector/spark-common/src/test/java/org/apache/gravitino/spark/connector/integration/test/SparkCommonIT.java:
##########
@@ -630,7 +630,8 @@ protected void testAlterTableReplaceColumns() {
tableInfo = getTableInfo(tableName);
checkTableColumns(tableName, updateColumns, tableInfo);
- sql(String.format("INSERT INTO %S VALUES(3, 'name2', 10)", tableName));
+ sql(String.format("CREATE TABLE IF NOT EXISTS %s (id int, name2 string,
age long)", tableName));
+ sql(String.format("INSERT INTO %s VALUES(3, 'name2', 10)", tableName));
Review Comment:
Fixed in 594b2d2e6e. The shared REPLACE COLUMNS test again performs both
CREATE TABLE IF NOT EXISTS and INSERT through an uppercase table name, so
native Hive, direct Iceberg-on-Hive, and REST-routing retain mixed-case
coverage. Only SparkIcebergCatalogRestBackendIT overrides the access to use the
original case, because a Gravitino catalog configured with catalog-backend=rest
cannot infer the downstream REST catalog's case-sensitivity; the REPLACE
COLUMNS assertions still run. REST-routing remains covered because its
Gravitino catalog is explicitly configured with catalog-backend=hive. I also
removed the now-redundant dedicated direct-Hive test. All four focused Spark
3.5 tests 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]
