wenzhenghu commented on code in PR #65126:
URL: https://github.com/apache/doris/pull/65126#discussion_r3694387762


##########
fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java:
##########
@@ -733,20 +728,16 @@ public void registerExternalTableFromEvent(String dbName, 
String tableName,
             return;
         }
 
-        long tblId = Util.genIdByName(catalogName, dbName, tableName);
-        // -1L means it will be dropped later, ignore
-        if (tblId == ExternalMetaIdMgr.META_ID_FOR_NOT_EXISTS) {
-            return;
-        }
+        ExternalDatabase<?> externalDatabase = (ExternalDatabase<?>) db;
+        String localTableName = 
externalDatabase.canonicalLocalTableNameFromRemote(tableName);

Review Comment:
   Fixed in `85776101dc8`.
   
   `HmsEventParser` now preserves the original table spelling from the HMS 
payload for `CREATE_TABLE` and the target of `ALTER_TABLE`; database 
normalization remains unchanged. Therefore, in mode 2 the event path registers 
the same exact remote table identity and ID as normal metadata loading, instead 
of introducing a lowercase alias. A later DROP using another Hive-equivalent 
spelling is resolved through the retained case-insensitive name index and 
removes that same identity.
   
   Regression coverage was added in `HmsEventParserTest` for CREATE/ALTER 
parsing and in `ExternalCatalogTest` for mode-2 CREATE followed by a 
differently-cased DROP.



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/MetastoreEventSyncDriver.java:
##########
@@ -289,14 +288,8 @@ private void applyRenameTable(PluginDrivenExternalCatalog 
catalog, MetastoreChan
             throws Exception {
         String catalogName = catalog.getName();
         CatalogMgr catalogMgr = Env.getCurrentEnv().getCatalogMgr();
-        // legacy AlterTableEvent: a rename to a DIFFERENT key that already 
exists locally is skipped
-        // (processRename guard); a view recreate (after == before) always 
proceeds (processRecreateTable).
-        boolean sameKey = 
descriptor.getDbName().equalsIgnoreCase(descriptor.getDbNameAfter())
-                && 
descriptor.getTableName().equalsIgnoreCase(descriptor.getTableNameAfter());
-        if (!sameKey && 
catalogMgr.externalTableExistInLocal(descriptor.getDbNameAfter(),
-                descriptor.getTableNameAfter(), catalogName)) {
-            return;
-        }
+        // Always converge to "old removed, new registered". The target may 
already be hot because a normal
+        // lookup raced with the event after the remote rename; it must not 
prevent cleanup of the old identity.
         catalogMgr.unregisterExternalTable(descriptor.getDbName(), 
descriptor.getTableName(),

Review Comment:
   Fixed in `85776101dc8`.
   
   Before applying a structural descriptor, `MetastoreEventSyncDriver` now 
invalidates the connector-owned cache using the descriptor remote names. 
CREATE/DROP invalidate the affected key; rename invalidates both source and 
target keys (deduplicated when they are identical); same-name DROP/CREATE 
reincarnations invalidate once for each event. Existing refresh and partition 
paths keep their current invalidation flow and are not duplicated.
   
   `MetastoreEventSyncDriverRenameTest` now verifies 
invalidation-before-FE-mutation ordering for table rename, same-name view 
replacement, same-name table reincarnation, and database reincarnation.



##########
fe/fe-core/src/main/java/org/apache/doris/datasource/plugin/PluginDrivenExternalCatalog.java:
##########
@@ -432,14 +443,16 @@ public void validateCreateTableEngine(String engineName) 
throws AnalysisExceptio
      * REGISTER_DATABASE change (via {@code 
CatalogMgr.registerExternalDatabaseFromEvent}). Pulled up from
      * {@code HMSExternalCatalog} so a flipped (generic) catalog no longer 
throws
      * {@code NotImplementedException} on a create/rename-database event. The 
body is fully generic
-     * (buildDbForInit + metaCache, name-derived id) and mirrors the legacy 
HMS implementation.
+     * (buildDbForInit + the shared metadata-cache update protocol) and 
mirrors the legacy HMS implementation.
      */
     @Override
-    public void registerDatabase(long dbId, String dbName) {
-        ExternalDatabase<? extends ExternalTable> db = buildDbForInit(dbName, 
null, dbId, logType, false);
+    public void registerDatabase(String dbName) {
+        String localDbName = canonicalLocalDatabaseNameFromRemote(dbName);

Review Comment:
   Fixed in `85776101dc8`.
   
   The driver now pins `getEventSource()` to the connector classloader, keeps 
`pollOnce()` under the event-source classloader, and runs descriptor 
application plus master-side full refresh under the connector classloader. The 
helper restores the caller TCCL in `finally`, including failure paths.
   
   A recording-loader `MetastoreEventSyncDriverClassLoaderTest` covers 
event-source acquisition, polling, descriptor invalidation/mutation, master 
full refresh, failure propagation, and caller-loader restoration.



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to